| [lsp4e-dev] LSP4e document corruption |
|
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.
I think the cause of this is in how didChange notifications are generated at https://github.com/eclipse/lsp4e/blob/6eefdb1413b0b043c76d9b6981edb6b87b5462b8/org.eclipse.lsp4e/src/org/eclipse/lsp4e/DocumentContentSynchronizer.java#L108-L109, which
is:
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).
I’ve also noticed that there are some other places in the codebase that seemingly suffer from similar issues: for example didSave and didChange need to appear in the right order but currently this is nothing to enforce this. Also, a request to
format the current document should happen only after the corresponding didChange notifications, and before other didChange notifications.
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.
Thanks,
Tom
|