Skip to content

Commit

Permalink
Fix race condition in LanguageServerWrapper resulting in NullPointerE…
Browse files Browse the repository at this point in the history
…xception during LS initialization (#985)

* Fix race condition resulting in NullPointerException

When running the test suite on Windows a lot of NPEs with the following
stacktrace are thrown:
```
java.lang.NullPointerException: Cannot invoke
"org.eclipse.core.runtime.IProgressMonitor.done()" because
"this.this$0.initializeFutureMonitor" is null
	at org.eclipse.lsp4e.LanguageServerWrapper$2.run(LanguageServerWrapper.java:407)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63)
```
and because of the NPE random tests seem to fail.

* improve method name advanceInitializeFutureMonitor

* use UncheckedIOException
  • Loading branch information
sebthom authored May 8, 2024
1 parent ca8ca6a commit 4e5a769
Showing 1 changed file with 16 additions and 13 deletions.
29 changes: 16 additions & 13 deletions org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -157,7 +158,7 @@ public void dirtyStateChanged(IFileBuffer buffer, boolean isDirty) {
protected StreamConnectionProvider lspStreamProvider;
private Future<?> launcherFuture;
private CompletableFuture<Void> initializeFuture;
private IProgressMonitor initializeFutureMonitor;
private final AtomicReference<IProgressMonitor> initializeFutureMonitorRef = new AtomicReference<>();
private final int initializeFutureNumberOfStages = 7;
private LanguageServer languageServer;
private LanguageClientImpl languageClient;
Expand Down Expand Up @@ -286,7 +287,7 @@ private synchronized void start(boolean forceRestart) throws IOException {
final Job job = createInitializeLanguageServerJob();
this.launcherFuture = new CompletableFuture<>();
this.initializeFuture = CompletableFuture.supplyAsync(() -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
if (LoggingStreamConnectionProviderProxy.shouldLog(serverDefinition.id)) {
this.lspStreamProvider = new LoggingStreamConnectionProviderProxy(
serverDefinition.createConnectionProvider(), serverDefinition.id);
Expand All @@ -297,11 +298,11 @@ private synchronized void start(boolean forceRestart) throws IOException {
try {
lspStreamProvider.start();
} catch (IOException e) {
throw new RuntimeException(e);
throw new UncheckedIOException(e);
}
return null;
}).thenRun(() -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
languageClient = serverDefinition.createLanguageClient();

initParams.setProcessId((int) ProcessHandle.current().pid());
Expand Down Expand Up @@ -333,18 +334,18 @@ private synchronized void start(boolean forceRestart) throws IOException {
this.launcherFuture = launcher.startListening();
})
.thenCompose(unused -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
return initServer(rootURI);
})
.thenAccept(res -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
serverCapabilities = res.getCapabilities();
this.initiallySupportsWorkspaceFolders = supportsWorkspaceFolders(serverCapabilities);
}).thenRun(() -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
this.languageServer.initialized(new InitializedParams());
}).thenRun(() -> {
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
final Map<URI, IDocument> toReconnect = filesToReconnect;
initializeFuture.thenRunAsync(() -> {
watchProjects();
Expand All @@ -357,7 +358,7 @@ private synchronized void start(boolean forceRestart) throws IOException {
}
});
FileBuffers.getTextFileBufferManager().addFileBufferListener(fileBufferListener);
advanceinitializeFutureMonitor();
advanceInitializeFutureMonitor();
}).exceptionally(e -> {
stop();
final Throwable cause = e.getCause();
Expand All @@ -378,7 +379,8 @@ private synchronized void start(boolean forceRestart) throws IOException {
}
}

private void advanceinitializeFutureMonitor() {
private void advanceInitializeFutureMonitor() {
final var initializeFutureMonitor = initializeFutureMonitorRef.get();
if (initializeFutureMonitor != null) {
if (initializeFutureMonitor.isCanceled()) {
throw new CancellationException();
Expand All @@ -387,11 +389,12 @@ private void advanceinitializeFutureMonitor() {
}
}

private synchronized Job createInitializeLanguageServerJob() {
private Job createInitializeLanguageServerJob() {
return new Job(NLS.bind(Messages.initializeLanguageServer_job, serverDefinition.label)) {
@Override
protected IStatus run(IProgressMonitor monitor) {
initializeFutureMonitor = SubMonitor.convert(monitor, initializeFutureNumberOfStages);
final var initializeFutureMonitor = SubMonitor.convert(monitor, initializeFutureNumberOfStages);
initializeFutureMonitorRef.set(initializeFutureMonitor);
CompletableFuture<Void> currentInitializeFuture = initializeFuture;
try {
if (currentInitializeFuture != null) {
Expand All @@ -405,7 +408,7 @@ protected IStatus run(IProgressMonitor monitor) {
}
} finally {
initializeFutureMonitor.done();
initializeFutureMonitor = null;
initializeFutureMonitorRef.compareAndSet(initializeFutureMonitor, null);
}
return Status.OK_STATUS;
}
Expand Down

0 comments on commit 4e5a769

Please sign in to comment.