Skip to content

Commit

Permalink
fix: IllegalStateException: Synchronizer should apply to only a single
Browse files Browse the repository at this point in the history
document, which is the one it was instantiated for

Fixes redhat-developer#752

Signed-off-by: azerr <[email protected]>
  • Loading branch information
angelozerr committed Aug 8, 2023
1 parent 768ccf0 commit b196c6c
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,9 @@
public class DocumentContentSynchronizer implements DocumentListener {
private final static Logger LOGGER = LoggerFactory.getLogger(DocumentContentSynchronizer.class);

private final @Nonnull
LanguageServerWrapper languageServerWrapper;
private final @Nonnull
Document document;
private final @Nonnull
URI fileUri;
private final @Nonnull LanguageServerWrapper languageServerWrapper;
private final @Nonnull Document document;
private final @Nonnull URI fileUri;
private final TextDocumentSyncKind syncKind;

private int version = 0;
Expand All @@ -61,7 +58,7 @@ public DocumentContentSynchronizer(@Nonnull LanguageServerWrapper languageServer
textDocument.setUri(fileUri.toString());
textDocument.setText(document.getText());

Language contentTypes = LSPIJUtils.getDocumentLanguage(this.document, languageServerWrapper.getProject());
Language contentTypes = LSPIJUtils.getDocumentLanguage(document, languageServerWrapper.getProject());

String languageId = languageServerWrapper.getLanguageId(contentTypes);

Expand All @@ -88,7 +85,6 @@ public void documentChanged(DocumentEvent event) {
if (syncKind == TextDocumentSyncKind.None) {
return;
}
checkEvent(event);
if (syncKind == TextDocumentSyncKind.Full) {
synchronized (changeEvents) {
changeEvents.clear();
Expand Down Expand Up @@ -118,7 +114,6 @@ private void sendDidChangeEvents() {

@Override
public void beforeDocumentChange(DocumentEvent event) {
checkEvent(event);
if (syncKind == TextDocumentSyncKind.Incremental) {
// this really needs to happen before event gets actually
// applied, to properly compute positions
Expand All @@ -129,6 +124,7 @@ public void beforeDocumentChange(DocumentEvent event) {
}

private TextDocumentContentChangeEvent createChangeEvent(DocumentEvent event) {
Document document = event.getDocument();
TextDocumentSyncKind syncKind = getTextDocumentSyncKind();
switch (syncKind) {
case None:
Expand Down Expand Up @@ -161,7 +157,9 @@ private TextDocumentContentChangeEvent createChangeEvent(DocumentEvent event) {
return null;
}

public void documentSaved(long timestamp) {
public void documentSaved(DocumentEvent event) {
long timestamp = event.getOldTimeStamp();
Document document = event.getDocument();
this.modificationStamp = timestamp;
ServerCapabilities serverCapabilities = languageServerWrapper.getServerCapabilities();
if (serverCapabilities != null) {
Expand Down Expand Up @@ -213,12 +211,4 @@ private void logDocument(String header, Document document) {
}
}

private void checkEvent(DocumentEvent event) {
if (this.document != event.getDocument()) {
logDocument("Listener document", this.document);
logDocument("Event document", event.getDocument());
throw new IllegalStateException("Synchronizer should apply to only a single document, which is the one it was instantiated for"); //$NON-NLS-1$
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void documentChanged(@NotNull DocumentEvent event) {
if (uri != null) {
DocumentContentSynchronizer documentListener = connectedDocuments.get(uri);
if (documentListener != null && documentListener.getModificationStamp() < event.getOldTimeStamp()) {
documentListener.documentSaved(event.getOldTimeStamp());
documentListener.documentSaved(event);
}
}
}
Expand Down Expand Up @@ -280,7 +280,10 @@ public synchronized void start() throws LanguageServerException {
return;
} else {
for (Map.Entry<URI, DocumentContentSynchronizer> entry : this.connectedDocuments.entrySet()) {
filesToReconnect.put(entry.getKey(), entry.getValue().getDocument());
// Get the Document from the VirtualFile to use the proper Document Instance
// (not an old instanceof the synchronizer which could be out of dated).
Document document = getDocument(entry.getKey(), entry.getValue());
filesToReconnect.put(entry.getKey(), document);
}
stop();
}
Expand All @@ -290,68 +293,68 @@ public synchronized void start() throws LanguageServerException {
final URI rootURI = getRootURI();
this.launcherFuture = new CompletableFuture<>();
this.initializeFuture = CompletableFuture.supplyAsync(() -> {
this.lspStreamProvider = serverDefinition.createConnectionProvider(initialProject.getProject());
initParams.setInitializationOptions(this.lspStreamProvider.getInitializationOptions(rootURI));
this.lspStreamProvider = serverDefinition.createConnectionProvider(initialProject.getProject());
initParams.setInitializationOptions(this.lspStreamProvider.getInitializationOptions(rootURI));

// Starting process...
udateStatus(ServerStatus.starting);
getLanguageServerLifecycleManager().onStatusChanged(this);
this.currentProcessId = null;
this.currentProcessCommandLines = null;
lspStreamProvider.start();

// As process can be stopped, we loose pid and command lines information
// when server is stopped, we store them here.
// to display them in the Language server explorer even if process is killed.
if (lspStreamProvider instanceof ProcessStreamConnectionProvider) {
ProcessStreamConnectionProvider provider = (ProcessStreamConnectionProvider) lspStreamProvider;
this.currentProcessId = provider.getPid();
this.currentProcessCommandLines = provider.getCommands();
}
// Starting process...
udateStatus(ServerStatus.starting);
getLanguageServerLifecycleManager().onStatusChanged(this);
this.currentProcessId = null;
this.currentProcessCommandLines = null;
lspStreamProvider.start();

// As process can be stopped, we loose pid and command lines information
// when server is stopped, we store them here.
// to display them in the Language server explorer even if process is killed.
if (lspStreamProvider instanceof ProcessStreamConnectionProvider) {
ProcessStreamConnectionProvider provider = (ProcessStreamConnectionProvider) lspStreamProvider;
this.currentProcessId = provider.getPid();
this.currentProcessCommandLines = provider.getCommands();
}

// Throws the CannotStartProcessException exception if process is not alive.
// This usecase comes for instance when the start process command fails (not a valid start command)
lspStreamProvider.ensureIsAlive();
return null;
}).thenRun(() -> {
languageClient = serverDefinition.createLanguageClient(initialProject.getProject());
initParams.setProcessId(getParentProcessId());

if (rootURI != null) {
initParams.setRootUri(rootURI.toString());
initParams.setRootPath(rootURI.getPath());
}
// Throws the CannotStartProcessException exception if process is not alive.
// This usecase comes for instance when the start process command fails (not a valid start command)
lspStreamProvider.ensureIsAlive();
return null;
}).thenRun(() -> {
languageClient = serverDefinition.createLanguageClient(initialProject.getProject());
initParams.setProcessId(getParentProcessId());

UnaryOperator<MessageConsumer> wrapper = consumer -> (message -> {
logMessage(message, consumer);
try {
// To avoid having some lock problem when message is written in the stream output
// (when there are a lot of messages to write it)
// we consume the message in async mode
CompletableFuture.runAsync(() -> consumer.consume(message));
} catch (Throwable e) {
// Log in the LSP console the error
getLanguageServerLifecycleManager().onError(this, e);
throw e;
}
final StreamConnectionProvider currentConnectionProvider = this.lspStreamProvider;
if (currentConnectionProvider != null && isActive()) {
currentConnectionProvider.handleMessage(message, this.languageServer, rootURI);
}
});
// initParams.setWorkspaceFolders(getRelevantWorkspaceFolders());
Launcher<LanguageServer> launcher = serverDefinition.createLauncherBuilder() //
.setLocalService(languageClient)//
.setRemoteInterface(serverDefinition.getServerInterface())//
.setInput(lspStreamProvider.getInputStream())//
.setOutput(lspStreamProvider.getOutputStream())//
.setExecutorService(listener)//
.wrapMessages(wrapper)//
.create();
this.languageServer = launcher.getRemoteProxy();
languageClient.connect(languageServer, this);
this.launcherFuture = launcher.startListening();
})
if (rootURI != null) {
initParams.setRootUri(rootURI.toString());
initParams.setRootPath(rootURI.getPath());
}

UnaryOperator<MessageConsumer> wrapper = consumer -> (message -> {
logMessage(message, consumer);
try {
// To avoid having some lock problem when message is written in the stream output
// (when there are a lot of messages to write it)
// we consume the message in async mode
CompletableFuture.runAsync(() -> consumer.consume(message));
} catch (Throwable e) {
// Log in the LSP console the error
getLanguageServerLifecycleManager().onError(this, e);
throw e;
}
final StreamConnectionProvider currentConnectionProvider = this.lspStreamProvider;
if (currentConnectionProvider != null && isActive()) {
currentConnectionProvider.handleMessage(message, this.languageServer, rootURI);
}
});
// initParams.setWorkspaceFolders(getRelevantWorkspaceFolders());
Launcher<LanguageServer> launcher = serverDefinition.createLauncherBuilder() //
.setLocalService(languageClient)//
.setRemoteInterface(serverDefinition.getServerInterface())//
.setInput(lspStreamProvider.getInputStream())//
.setOutput(lspStreamProvider.getOutputStream())//
.setExecutorService(listener)//
.wrapMessages(wrapper)//
.create();
this.languageServer = launcher.getRemoteProxy();
languageClient.connect(languageServer, this);
this.launcherFuture = launcher.startListening();
})
.thenCompose(unused -> initServer(rootURI))
.thenAccept(res -> {
serverError = null;
Expand Down Expand Up @@ -396,6 +399,25 @@ public synchronized void start() throws LanguageServerException {
}
}

/**
* Returns the document instance from the virtual file which matches the given uri and the document from teh synchronizer otherwise.
*
* @param uri the document Uri
* @param synchronizer the document synchronizer.
* @return the document instance from the virtual file which matches the given uri and the document from teh synchronizer otherwise.
*/
private static Document getDocument(URI uri, DocumentContentSynchronizer synchronizer) {
Document document = synchronizer.getDocument();
VirtualFile file = LSPIJUtils.findResourceFor(uri);
if (file != null) {
Document documentFromFile = LSPIJUtils.getDocument(file);
if (documentFromFile != null) {
document = documentFromFile;
}
}
return document;
}

private CompletableFuture<InitializeResult> initServer(final URI rootURI) {

final var workspaceClientCapabilities = SupportedFeatures.getWorkspaceClientCapabilities();
Expand Down Expand Up @@ -487,7 +509,7 @@ public boolean isActive() {

@Override
public void dispose() {
this.disposed= true;
this.disposed = true;
stop();
stopDispatcher();
}
Expand Down Expand Up @@ -819,7 +841,13 @@ private void disconnect(URI path) {
private void disconnect(URI path, boolean stopping) {
DocumentContentSynchronizer documentListener = this.connectedDocuments.remove(path);
if (documentListener != null) {
// Remove teh listener from the old document stored in synchronizer
documentListener.getDocument().removeDocumentListener(documentListener);
Document document = getDocument(path, documentListener);
if (document != documentListener.getDocument()) {
// It should never occur, but to be sure we remove the document listener from the document of the VirtualFile
document.removeDocumentListener(documentListener);
}
documentListener.documentClosed();
}
if (!stopping && this.connectedDocuments.isEmpty()) {
Expand Down

0 comments on commit b196c6c

Please sign in to comment.