Skip to content
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.

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

LSP: Prevent deadlock with synchronous client #237

Closed
wants to merge 1 commit into from
Closed

Conversation

strager
Copy link
Collaborator

@strager strager commented Apr 18, 2021

If an LSP client sends a bunch of requests, but does not read from
quick-lint-js' stdout, quick-lint-js can hang in pipe_reader::write.
(This came up when writing some benchmarks. My LSP client naively made a
bunch of requests without reading responses during the requests.)

Prevent this (unlikely) deadlock by switching stdout to non-blocking
mode, buffering data in memory, and resuming reading from stdin as soon
as possible.

@strager strager mentioned this pull request Apr 18, 2021
@strager strager linked an issue Apr 18, 2021 that may be closed by this pull request
@@@ winders
@@@ perf. lots of copying. sadge. =[
@@@ test LSP server manually

If an LSP client sends a bunch of requests, but does not read from
quick-lint-js' stdout, quick-lint-js can hang in pipe_reader::write.
(This came up when writing some benchmarks. My LSP client naively made a
bunch of requests without reading responses during the requests.)

Prevent this (unlikely) deadlock by switching stdout to non-blocking
mode, buffering data in memory, and resuming reading from stdin as soon
as possible.

The current implementation is relatively inefficient. Data is copied
many times. Maybe we'll clean this up in the future.
@strager
Copy link
Collaborator Author

strager commented Apr 19, 2021

I'm going to punt on this PR. Fixing this issue isn't worth the complexity (Linux) or the [theorized] performance hit (Windows). Saved my progress to refs/archive/lsp-deadlock.

@strager strager closed this Apr 19, 2021
@strager strager deleted the lsp-deadlock branch April 19, 2021 02:02
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.

LSP: Pipes get clogged
1 participant