lsp: incorrect symbol positions

[kitsonk]

I noticed when editing documents, sometimes in larger documents, I started to see "incorrect" symbol positions in the editor. I believe this is because of the async nature of the language server, but the sync nature of tsc. I believe the symbols were starting to be calculated while additional text edits were occurring in the document, which resulted in the response positions for the symbols being off a few positions.

What we need to do is the version of the document before we go into tsc and then discard the results from tsc if the version of the document has changed, and re-run again with the new version until we get a matching pre and post versions. I guess we should also check into the cancellation API as well.

[ebkalderon]

@kitsonk Author of tower-lsp here, the original LSP framework from which lspower derives. In addition to the inherent difficulties in maintaining consistent language server state that you've outlined, I believe there also exists a deeper issue within both LSP frameworks that makes this sort of downstream bug more common, as described in ebkalderon/tower-lsp#284. There may be a few things that can be done on the framework side as well as the application side to improve the situation somewhat. Any input would be most appreciated!

[kitsonk]

Yeah, reading that issue, I could see how it is "easy" to run into bugs like that. We have also had some dead-locking issues which are likely related to this space as well, which have made us have to worry about more of our locking mechanisms when processing things on different threads. I will look at the issue in more detail and provide some thoughts feedback. Thanks for reaching out.

[ebkalderon]

No worries! I think that gathering feedback and recounting real-world experiences from framework users and language server implementers would be very valuable here. It would help us determine whether concurrent execution really contributes to these classes of bugs, and if so, we can restructure Server such that they are somewhat less likely to occur (the exact degree of which is difficult to estimate ahead of time, since the blame might lie in either the framework, the server built on top of it, or both).

[ebkalderon]

Release v0.16.0 of tower-lsp was published about a week ago, and it adds a concurrency_level() setter to Server, which lets users adjust the maximum number of client-to-server requests in-flight from 4 (the default value so far), as needed. As such, it should now be possible to set the concurrency level to 1 and observe whether these incorrect symbol positions disappear.

However, this would be a pretty brute-force solution. If concurrent request handling is still desirable, versioning the document data and discarding outdated information, as suggested in the original ticket description, would be much more palatable. Hopefully making progress on ebkalderon/tower-lsp#284 will improve things further.

[nayeemrmn]

We account for versions and have cancellations for lsp/tsc now.