Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [lsp4e-dev] LSP4e document corruption


On Thu, Mar 17, 2022 at 12:20 PM Thomas Gibson-Robinson via lsp4e-dev <lsp4e-dev@xxxxxxxxxxx> wrote:
Hi all,

One thing that we’ve noticed when using LSP4e is that the server’s view on documents appears to very occasionally get out of sync with a client’s. This can manifest itself either by Eclipse showing problems for old versions of the file, or showing seemingly random error messages that indicate the server has a completely different view of the editor’s content. This seems to be more common when the system is under high load.

Right, I also see this issue occasionally and it was also mentioned a few other times by other users.
 
languageServerWrapper.getInitializedServer()
.thenAcceptAsync(ls -> ls.getTextDocumentService().didChange(changeParamsToSend));

My understanding of Java futures is that unless a special executor is used, there is no guarantee about the order in which futures are executed, even when the futures are created by a single thread. Therefore, the body of the above lambda (i.e. the didChange call) could, in theory, be executed out-of-order with respect to other calls to the language server, including other didChange notifications. This could lead to the server applying the modifications in the wrong order, leading to the server’s version of the document becoming corrupted (in the case of didChange sending diffs), or to be the wrong version (in the case of didChange sending full file contents each time).

That analysis of the problem and its cause seems correct.

Does this seem like a genuine problem with LSP4e, or am I missing something about Java futures that means the FIFO order is otherwise guaranteed? If this does seem like a genuine problem, would a patch that enforces FIFO ordering on some requests be welcome? This would probably mean sequentialising didChange, didSave and formatting-related requests, although I do wonder if (almost?) all requests should be included in some linear ordering, given that the editor probably wants (e.g.) code lenses or code completion for the current document version, not the version that will be created in the future after the user types some extra content.

It's a genuine problem of LSP4E.
However, I don't think you need a FIFO. You actually need to keep the last future that was created for a change and call something like `lastFuture = lastFuture.thenRun(ls -> ls.getTextDocumentService().didChange(changeParamsToSend));` to ensure the sequential invocations. Maybe even replace `languageServerWrapper.getInitializedServer()` by a method that takes care of returning the last change future.

Back to the top