Skip to content

Fix broken pipe during test and gracefully exit the server#4701

Merged
soulomoon merged 1 commit into
masterfrom
1875-tests-randomly-fail-with-exception-fd111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround
Mar 10, 2026
Merged

Fix broken pipe during test and gracefully exit the server#4701
soulomoon merged 1 commit into
masterfrom
1875-tests-randomly-fail-with-exception-fd111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround

Conversation

@soulomoon
Copy link
Copy Markdown
Collaborator

@soulomoon soulomoon commented Aug 21, 2025

gracefully exit the server

@soulomoon soulomoon linked an issue Aug 21, 2025 that may be closed by this pull request
Tests randomly fail with "Exception: fd:111: hPutBuf: resource vanished (Broken pipe)" (--test-option=-j1 workaround) #1875
Closed
@soulomoon soulomoon changed the title Fix broken pip during test Fix broken pipe during test Aug 22, 2025
@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Aug 22, 2025

Key idea:

  1. Wait for the refactor loop to finish in shutdown handler.
  2. After sending the shutdown response, stop sending further response to client. (This is done in lsp)

@soulomoon soulomoon mentioned this pull request Aug 26, 2025
Merged
@soulomoon soulomoon changed the title Fix broken pipe during test Fix broken pipe during test and gracefully exit the server Aug 26, 2025
@soulomoon soulomoon force-pushed the 1875-tests-randomly-fail-with-exception-fd111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround branch from eb5356d to 773bfee Compare September 5, 2025 18:28
@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Sep 18, 2025 •
edited
Loading

What's been done

  1. Refactor exit logic

    • Previously, shutdown and Shake database cleanup only happened in the shutdown handler.
    • Now, the cleanup is moved into the finally clause of the reactor thread, ensuring it always runs on reactor exit.
    • Also adapted to a modified LSP package branch so the server can now gracefully exit. Gracefully exit the server lsp#622 (comment)
    • Workaround the case that workthread might not exit because the threadcancel exception is swollen by downstream code.

  2. Improve logging

    • Added clearer and more structured logging to aid debugging.

  3. Add flakiness testing support

    • Introduced a flakiness testing script and CI integration.
    • Makes it easier to detect and diagnose flaky behavior in development and CI runs.

@soulomoon soulomoon marked this pull request as ready for review September 18, 2025 12:45
@soulomoon soulomoon added the status: needs review This PR is ready for review label Sep 20, 2025
@fendor
Copy link
Copy Markdown
Collaborator

fendor commented Nov 24, 2025

@soulomoon Is this ready for review?

@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Nov 24, 2025

@soulomoon Is this ready for review?

yes, it is. thanks.

@soulomoon soulomoon reopened this Nov 26, 2025
@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Dec 17, 2025

odd 9.10 windows failing for Access violation in generated code when reading 0x7ff4933977a0

@fendor
Copy link
Copy Markdown
Collaborator

fendor commented Dec 18, 2025 •
edited
Loading

@soulomoon Probably just needs to rebased on top of #4768

@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Jan 14, 2026

ci have been fixed except circleci. now waiting for new lsp version to be released.

This was referenced Feb 9, 2026
Merged
Merged
Aster89
Aster89 reviewed Feb 19, 2026
View reviewed changes
Comment thread cabal.project Outdated
fendor
fendor requested changes Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some residual files that we likely should get rid of

Comment thread .github/workflows/flakiness.yml Outdated
Comment thread scripts/eventlog_dump.py Outdated
fendor
fendor reviewed Mar 2, 2026
View reviewed changes
Comment thread scripts/flaky-test-loop.sh Outdated
fendor
fendor reviewed Mar 2, 2026
View reviewed changes
Comment thread scripts/flaky-test-patterns.txt Outdated
@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Mar 4, 2026 •
edited
Loading

windows will still see some broken pipes, but it is not caused by the same gracefully exit issue.
For ubuntu pipeline, broken pipes are gone.
@fendor I've clean up the debug files. This patch is ready.

fendor
fendor requested changes Mar 7, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks like a big improvement!

I have added a couple of comments, let me know what you think about it and whether I should address any of them myself.

Comment thread ghcide/src/Development/IDE/Core/WorkerThread.hs Outdated
Comment thread ghcide/src/Development/IDE/Core/WorkerThread.hs
Comment thread ghcide/src/Development/IDE/Core/WorkerThread.hs Outdated
Comment thread ghcide/src/Development/IDE/Core/WorkerThread.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
fendor
fendor requested changes Mar 7, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks like a big improvement!

I have added a couple of comments, let me know what you think about it and whether I should address any of them myself.

fendor
fendor reviewed Mar 7, 2026
View reviewed changes
Comment thread ghcide/src/Development/IDE/Core/WorkerThread.hs Outdated
@soulomoon soulomoon requested a review from fendor March 8, 2026 09:21
@soulomoon
Copy link
Copy Markdown
Collaborator Author

soulomoon commented Mar 8, 2026

@fendor Thanks for the review! I’ve addressed the comments and pushed the updates. Please take another look when you have time.

fendor
fendor approved these changes Mar 8, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for this marathon of a PR :D

Excited to finally have more of your improvements merged into HLS.

@fendor
Copy link
Copy Markdown
Collaborator

fendor commented Mar 8, 2026

Feel free to merge when you are ready

@soulomoon soulomoon force-pushed the 1875-tests-randomly-fail-with-exception-fd111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround branch from 6b5fe23 to 402ce40 Compare March 9, 2026 16:10
@soulomoon soulomoon enabled auto-merge (squash) March 9, 2026 16:12
@soulomoon @fendor
Prevent broken pipes and shutdown hangs during LSP shutdown
df6b45f 
Coordinate LanguageServer shutdown with worker queue teardown. The reactor now requests shutdown, waits for confirmation, logs timeout and exit outcomes, and stops server-side work before late background tasks can outlive the server. Worker threads also stop dequeuing after shutdown is requested, preventing broken-pipe failures and shutdown hangs in tests.

Bump the lsp lower bound to 2.8 to match the shutdown flow used here.

Co-authored-by: fendor <fendor@users.noreply.github.com>
@soulomoon soulomoon force-pushed the 1875-tests-randomly-fail-with-exception-fd111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround branch from 402ce40 to df6b45f Compare March 9, 2026 16:20
@soulomoon soulomoon merged commit 298cf98 into master Mar 10, 2026
52 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fendor fendor fendor approved these changes

@michaelpj michaelpj Awaiting requested review from michaelpj michaelpj is a code owner

@jian-lin jian-lin Awaiting requested review from jian-lin

@wz1000 wz1000 Awaiting requested review from wz1000 wz1000 is a code owner

+1 more reviewer

@Aster89 Aster89 Aster89 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: needs review This PR is ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tests randomly fail with "Exception: fd:111: hPutBuf: resource vanished (Broken pipe)" (--test-option=-j1 workaround)

3 participants

@soulomoon @fendor @Aster89