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

Server side LSP request hangs if executed from a notification handler #775

Closed
MiklosMagyari opened this issue Dec 15, 2023 · 5 comments
Closed
Milestone

Comments

@MiklosMagyari
Copy link

MiklosMagyari commented Dec 15, 2023

I don't know if it is by design, but at least it was not trivial for me and took several hours to track down.

We have a language server built on lsp4j. Among many others, we implemented the handler for didChangeWorkspaceFolders. After handling the folder changes, we call the function performing a project build that creates a work progress:

@Override
public void didChangeWorkspaceFolders(DidChangeWorkspaceFoldersParams params) {
    ...
    build();  // at a point it sends work done progress create
    ...
}

It turned out that calling the build like this causes sending window/workDoneProgress/create request to hang. Checking the trace in vscode showed that the request has been sent out and vscode responded almost immediatelly, but the CompletableFuture returned by createProgress() did not complete.

Finally, I have changed the code to:

@Override
public void didChangeWorkspaceFolders(DidChangeWorkspaceFoldersParams params) {
    ...
    CompletableFuture.runAsync(() -> {
        build();
    });
    ...
}

This way I have no lockup.

Is it intentional that I practically need to exit notification handlers before sending out an LSP request?

@pisv
Copy link
Contributor

pisv commented Dec 15, 2023

@MiklosMagyari Thank you for the detailed report.

Basically, LSP4J runs a loop in a dedicated thread that reads messages from an input stream and dispatches them to the corresponding handlers (for details, see StreamMessageProducer). Therefore, it is generally not a good idea to implement the handlers for requests or notifications in a blocking, synchronous way.

In your specific case, the deadlock occurred because the thread that reads and dispatches incoming messages was blocked by synchronously waiting for the response to the window/workDoneProgress/create request and, hence, was not able to read that response in the first place.

HTH

@MiklosMagyari
Copy link
Author

Thanks a lot for the clarification, that explains why this lockup happened.

Btw, is this documented somewhere? Maybe I have missed it?
I believe this information would be useful for others as well.

pisv added a commit that referenced this issue Dec 17, 2023
Updated documentation based on questions asked in #775 and #770.
@pisv
Copy link
Contributor

pisv commented Dec 17, 2023

@MiklosMagyari I have opened PR #777 to update the documentation.

Any comments are welcome!

@MiklosMagyari
Copy link
Author

Great! Short and concise, can spare hours of debugging. Thanks once again.

pisv added a commit that referenced this issue Dec 20, 2023
Updated documentation based on questions asked in #775 and #770.
@pisv
Copy link
Contributor

pisv commented Dec 20, 2023

@MiklosMagyari Thanks a lot for your feedback!

I have merged the PR.

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

No branches or pull requests

2 participants