Skip to content

Keep communication pipes between lsp-test and ghcide alive for the entire test#4892

Merged
crtschin merged 1 commit intohaskell:masterfrom
crtschin:crtschin/keep-test-handles-alive
Apr 13, 2026
Merged

Keep communication pipes between lsp-test and ghcide alive for the entire test#4892
crtschin merged 1 commit intohaskell:masterfrom
crtschin:crtschin/keep-test-handles-alive

Conversation

@crtschin
Copy link
Copy Markdown
Collaborator

@crtschin crtschin commented Apr 10, 2026

Closes #4884.

Because the pipe is split up into separate handles and sent off to the separate component calls to lsp-test (async through ghcide in in-thread to lsp-test directly to communicate the test messages), a GC when either is in the process of being shutdown, will close one end of the pipe. This forms a race condition where the other end of the pipe hasn't been terminated and now unexpectedly reads a EOF.

For a more comprehensive explanation see my previous attempt at fixing this via lsp-test by being lenient on shutdown. I think this PR is nicer compared to that one, as it tackles the core issue, the handles being GC'd prematurely.

@crtschin crtschin force-pushed the crtschin/keep-test-handles-alive branch 2 times, most recently from 12c5da3 to 1275248 Compare April 10, 2026 20:22
@crtschin
Copy link
Copy Markdown
Collaborator Author

So on the plus side, I don't see errors related to a closed handle anymore (no Language server unexpectedly terminated or failed hPutBuf: resource vanished). But the tests are still flaky with lsp message timeouts and errors related to progress reporting. Those are documented here though. Calling this a win, and leaving the remaining flaky failures for a follow up.

@crtschin crtschin force-pushed the crtschin/keep-test-handles-alive branch from 1275248 to b636323 Compare April 10, 2026 21:26
@soulomoon
Copy link
Copy Markdown
Collaborator

soulomoon commented Apr 11, 2026

@crtschin Thanks for trying to improve the flaky test situation.
Regarding to progress report problem, here is some context might be helpful #4714

@crtschin crtschin force-pushed the crtschin/keep-test-handles-alive branch from b636323 to 79b0a04 Compare April 12, 2026 18:48
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!
In the comment, could you explain where the read is happening?
It doesn't look trivial.

@crtschin crtschin marked this pull request as ready for review April 13, 2026 11:12
@crtschin crtschin force-pushed the crtschin/keep-test-handles-alive branch from 79b0a04 to 03b823e Compare April 13, 2026 11:12
@crtschin crtschin merged commit 1055d5b into haskell:master Apr 13, 2026
71 of 76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flakiness on running ghcide-tests due to ghcide + lsp-tests

3 participants