-
Notifications
You must be signed in to change notification settings - Fork 92
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
Ark: LSP crash while looking for completions or creating new document context #2692
Comments
Caught one while typing in the console in a pretty fresh Positron session. Here's everything in my Console that popped up as I was typing If I had to guess, maybe the Say the document is at
But the Note to self - I thought it was impossible for
Full Console output below. Nothing useful in LSP output.
|
@lionel- and I think this may be heavily related to ebkalderon/tower-lsp#284 i.e. we are guessing that the frontend is actually sending the requests over in sequential order, but tower-lsp has an async request processing framework in place that could result in us finally receiving those requests out of order. This may also explain why sometimes we get It seems to be a pretty big issue, with lots of comments on that thread. ruff also went as far as implementing their own server rather than using tower-lsp to avoid this potential issue |
Actually the tower-lsp issue is only affecting response ordering, AFAIU. The handlers for incoming messages should be called in message order as expected and it's up to us to ensure that the concurrency doesn't mess up handling expectations. I think we can achieve that with an appropriate locking strategy. Edit: Actually we should also be able to use some locking setup on handler exit to queue responses in the incoming order. This should fix the issue discussed in the linked thread. |
@DavisVaughan I moved this to Ready for verification, but feel free to close since I believe you've been using the lsp-sync for a while and haven't observed any crashes. |
Yea I'm going to close and we can reopen another issue if we ever discover this again, but hopefully not 🤞 |
Here is the state of my positron window when the crash happened. I was typing in the console IIRC:
Complete logs:
Relevant part of the log (why is the leading json truncated like this?):
Unfortunately the backtrace is missing from the logs for some reason. The crash happens here, we might want to replace the
unwrap()
with some logging:https://github.com/posit-dev/amalthea/blob/ce635139ac93f22a12316710e640d601c80981cd/crates/ark/src/lsp/document_context.rs#L28
Slack threads:
The text was updated successfully, but these errors were encountered: