Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 64 commits into
base: master
Choose a base branch
from
Open

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

wants to merge 64 commits into from
+733 −112

Conversation

soulomoon
Copy link
Collaborator

gracefully exit the server

@soulomoon
Fix broken pip during test
ae3a5f1 
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
Open
@soulomoon soulomoon changed the title Fix broken pip during test Fix broken pipe during test Aug 22, 2025
soulomoon added 12 commits August 22, 2025 08:35
@soulomoon
Update flakiness.yml
e1a7947
@soulomoon
Add InitParameters data type and enhance shutdown handling in Languag…
ed85d9b 
…eServer
@soulomoon
Rename InitParameters to InitializationContext and update related fie…
100b39e 
…lds for clarity
@soulomoon
Increase default maximum iterations to 1000 in flakiness workflow
5835ad7
@soulomoon
Update cabal.project and LanguageServer for improved logging and upda…
d26ed7f 
…te to use new lsp revision
@soulomoon
Merge branch 'master' into 1875-tests-randomly-fail-with-exception-fd…
c0f6a9b 
…111-hputbuf-resource-vanished-broken-pipe---test-option-j1-workaround
@soulomoon
Update lsp repository tag to latest commit
1f9cb02
@soulomoon
Improve log message for server exit and simplify test failure detecti…
c72d2e7 
…on pattern
@soulomoon
Fix flakiness test
569d766
@soulomoon
Set default max_iter value to 1000 in flakiness test workflow
cb67ec5
@soulomoon
Refactor logging in open-close loop script to improve iteration outpu…
56bc03b 
…t and progress reporting
@soulomoon
Fix exit codes for broken pipe and test failure detection in open-clo…
9f24f2e 
…se loop script
@soulomoon
Copy link
Collaborator Author

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
Open
@soulomoon
update flaky-test-loop script
6907be0
@soulomoon
update lsp rev
0851914
@soulomoon
Use a TMVar as a stop flag to coordinate graceful shutdown.
7900d71 
The worker thread checks this flag before dequeuing each job; if set, it exits immediately,
ensuring that no new work is started after shutdown is requested.
This mechanism is necessary because some downstream code may swallow async exceptions,
making 'cancel' unreliable for stopping the thread in all cases.
If 'cancel' does interrupt the thread (e.g., while blocked in STM or in a cooperative job),
the thread exits immediately and never checks the TMVar; in such cases, the stop flag is redundant.
@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
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)

  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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

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

@jian-lin jian-lin Awaiting requested review from jian-lin jian-lin is a code owner

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

At least 1 approving review is required to merge this pull request.

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)
1 participant
@soulomoon