Skip to content

Commit

Permalink
Stop language server on shutdown (#681)
Browse files Browse the repository at this point in the history 
Note the use of Plafotm.getLog() - ILog can't be received from plug-in
activator during platform shutdown.
  • Loading branch information
@basilevs
basilevs committed Jun 7, 2023
1 parent 97cb4e3 commit f2967cb
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 56 deletions.
54 changes: 51 additions & 3 deletions ...lsp4e/test/LanguageServerWrapperTest.java → ...ipse/lsp4e/LanguageServerWrapperTest.java
View file
Expand Up @@ -9,21 +9,30 @@
* Contributors:
* Markus Ofterdinger (SAP SE) - initial implementation
*******************************************************************************/
package org.eclipse.lsp4e.test;
package org.eclipse.lsp4e;

import static org.eclipse.lsp4e.test.TestUtils.waitForAndAssertCondition;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.logging.Handler;
import java.util.logging.LogRecord;
import java.util.logging.Logger;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.lsp4e.LanguageServerWrapper;
import org.eclipse.lsp4e.LanguageServiceAccessor;
import org.eclipse.lsp4e.test.AllCleanRule;
import org.eclipse.lsp4e.test.TestUtils;
import org.eclipse.lsp4e.tests.mock.MockLanguageServerMultiRootFolders;
import org.eclipse.lsp4j.jsonrpc.json.StreamMessageProducer;
import org.eclipse.ui.IEditorPart;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -63,4 +72,43 @@ public void testConnect() throws Exception {
TestUtils.closeEditor(editor1, false);
TestUtils.closeEditor(editor2, false);
}

@Test
public void doNotStopBusyDispatchers() throws Exception {
final Logger LOG = Logger.getLogger(StreamMessageProducer.class.getName());
List<String> logMessages = Collections.synchronizedList(new ArrayList<>());
LOG.addHandler(new Handler() {

@Override
public void publish(LogRecord record) {
logMessages.add(record.getMessage());
}

@Override
public void flush() {

}

@Override
public void close() throws SecurityException {
}
});
IFile testFile1 = TestUtils.createFile(project1, "shouldUseExtension.lsptWithMultiRoot", "");
IEditorPart editor1 = TestUtils.openEditor(testFile1);

try {
@NonNull Collection<LanguageServerWrapper> wrappers = LanguageServiceAccessor.getLSWrappers(testFile1, request -> true);
LanguageServerWrapper wrapper = wrappers.iterator().next();
assertEquals(1, wrappers.size());
waitForAndAssertCondition(2_000, () -> MockLanguageServerMultiRootFolders.INSTANCE.isRunning());
assertTrue(wrapper.isConnectedTo(testFile1.getLocationURI()));
logMessages.clear();
wrapper.stopDispatcher();
waitForAndAssertCondition(2_000, () -> !MockLanguageServerMultiRootFolders.INSTANCE.isRunning());
Assert.assertEquals(Collections.emptyList(), logMessages);
} finally {
TestUtils.closeEditor(editor1, false);
}

}
}
1 change: 1 addition & 0 deletions org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/AllTests.java
View file
Expand Up @@ -12,6 +12,7 @@
*******************************************************************************/
package org.eclipse.lsp4e.test;

import org.eclipse.lsp4e.LanguageServerWrapperTest;
import org.eclipse.lsp4e.test.codeactions.CodeActionTests;
import org.eclipse.lsp4e.test.color.ColorTest;
import org.eclipse.lsp4e.test.commands.DynamicRegistrationTest;
Expand Down
5 changes: 4 additions & 1 deletion ...lsp4e.tests.mock/src/org/eclipse/lsp4e/tests/mock/MockLanguageServerMultiRootFolders.java
View file
Expand Up @@ -137,7 +137,10 @@ public U apply(Void v) {

@Override
public CompletableFuture<InitializeResult> initialize(InitializeParams params) {
return buildMaybeDelayedFuture(initializeResult);
return buildMaybeDelayedFuture(initializeResult).thenApply(result -> {
started = true;
return result;
});
}

@Override
Expand Down
146 changes: 94 additions & 52 deletions org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerWrapper.java
View file
Expand Up @@ -35,7 +35,6 @@
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;
Expand All @@ -54,6 +53,7 @@
import org.eclipse.core.resources.WorkspaceJob;
import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.ILog;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
Expand Down Expand Up @@ -111,6 +111,8 @@

public class LanguageServerWrapper {

private static final ILog LOG = Platform.getLog(LanguageServerWrapper.class);

private final IFileBufferListener fileBufferListener = new FileBufferListenerAdapter() {
@Override
public void bufferDisposed(IFileBuffer buffer) {
Expand Down Expand Up @@ -158,7 +160,7 @@ public void dirtyStateChanged(IFileBuffer buffer, boolean isDirty) {
private ServerCapabilities serverCapabilities;
private final Timer timer = new Timer("Stop Language Server Task Processor"); //$NON-NLS-1$
private TimerTask stopTimerTask;
private AtomicBoolean stopping = new AtomicBoolean(false);
private final AtomicReference<CompletableFuture<Void>> stopping = new AtomicReference<CompletableFuture<Void>>(null);

private final ExecutorService dispatcher;

Expand Down Expand Up @@ -201,14 +203,26 @@ private LanguageServerWrapper(@Nullable IProject project, @NonNull LanguageServe
}

void stopDispatcher() {
this.dispatcher.shutdownNow();

// Only really needed for testing - the listener (an instance of ConcurrentMessageProcessor) should exit
// as soon as the input stream from the LS is closed, and a cached thread pool will recycle idle
// threads after a 60 second timeout - or immediately in response to JVM shutdown.
// If we don't do this then a full test run will generate a lot of threads because we create new
// instances of this class for each test
this.listener.shutdownNow();
if (dispatcher.isShutdown()) {
return;
}
try {
stop().get(6, TimeUnit.SECONDS);
} catch (InterruptedException e ) {
Thread.currentThread().interrupt();
LOG.error(e.getLocalizedMessage(), e);
} catch (ExecutionException | TimeoutException e) {
LOG.error(String.format("Failed to stop %s in 6 second", this), e); //$NON-NLS-1$
} finally {
this.dispatcher.shutdownNow();

// Only really needed for testing - the listener (an instance of ConcurrentMessageProcessor) should exit
// as soon as the input stream from the LS is closed, and a cached thread pool will recycle idle
// threads after a 60 second timeout - or immediately in response to JVM shutdown.
// If we don't do this then a full test run will generate a lot of threads because we create new
// instances of this class for each test
this.listener.shutdownNow();
}
}

/**
Expand Down Expand Up @@ -241,34 +255,54 @@ private List<WorkspaceFolder> getRelevantWorkspaceFolders() {
*/
public synchronized void start() throws IOException {
final var filesToReconnect = new HashMap<URI, IDocument>();
final CompletableFuture<?> stopFuture;
if (this.languageServer != null) {
if (isActive()) {
return;
} else {
for (Entry<URI, DocumentContentSynchronizer> entry : this.connectedDocuments.entrySet()) {
filesToReconnect.put(entry.getKey(), entry.getValue().getDocument());
}
stop();
stopFuture = stop();
}
} else {
stopFuture = CompletableFuture.completedFuture(null);
}
if (this.initializeFuture == null) {
final URI rootURI = getRootURI();
this.launcherFuture = new CompletableFuture<>();
this.initializeFuture = CompletableFuture.supplyAsync(() -> {
this.initializeFuture = stopFuture.thenCompose(ignored -> initialize());
initializeFuture.thenRunAsync(() -> {
FileBuffers.getTextFileBufferManager().addFileBufferListener(fileBufferListener);
watchProjects();
for (Entry<URI, IDocument> fileToReconnect : filesToReconnect.entrySet()) {
try {
connect(fileToReconnect.getKey(), fileToReconnect.getValue());
} catch (IOException e) {
throw new RuntimeException(e);
}
}
});
}
}

private CompletableFuture<Void> initialize() {
final URI rootURI = getRootURI();
return CompletableFuture.supplyAsync(() -> {
synchronized (this) {
if (LoggingStreamConnectionProviderProxy.shouldLog(serverDefinition.id)) {
this.lspStreamProvider = new LoggingStreamConnectionProviderProxy(
serverDefinition.createConnectionProvider(), serverDefinition.id);
} else {
this.lspStreamProvider = serverDefinition.createConnectionProvider();
}
initParams.setInitializationOptions(this.lspStreamProvider.getInitializationOptions(rootURI));
try {
lspStreamProvider.start();
} catch (IOException e) {
throw new RuntimeException(e);
}
return null;
}).thenRun(() -> {
}
try {
lspStreamProvider.start();
} catch (IOException e) {
throw new RuntimeException(e);
}
synchronized (this) {
languageClient = serverDefinition.createLanguageClient();

initParams.setProcessId((int) ProcessHandle.current().pid());
Expand Down Expand Up @@ -298,33 +332,20 @@ public synchronized void start() throws IOException {
this.languageServer = launcher.getRemoteProxy();
languageClient.connect(languageServer, this);
this.launcherFuture = launcher.startListening();
})
.thenCompose(unused -> initServer(rootURI))
.thenAccept(res -> {
}
return null;
}, dispatcher).thenCompose(unused -> initServer(rootURI)).thenAccept(res -> {
synchronized(this) {
serverCapabilities = res.getCapabilities();
this.initiallySupportsWorkspaceFolders = supportsWorkspaceFolders(serverCapabilities);
}).thenRun(() -> {
this.languageServer.initialized(new InitializedParams());
}).thenRun(() -> {
final Map<URI, IDocument> toReconnect = filesToReconnect;
initializeFuture.thenRunAsync(() -> {
watchProjects();
for (Entry<URI, IDocument> fileToReconnect : toReconnect.entrySet()) {
try {
connect(fileToReconnect.getKey(), fileToReconnect.getValue());
} catch (IOException e) {
throw new RuntimeException(e);
}
}
});
FileBuffers.getTextFileBufferManager().addFileBufferListener(fileBufferListener);
}).exceptionally(e -> {
LanguageServerPlugin.logError(e);
initializeFuture.completeExceptionally(e);
stop();
return null;
});
}
}
}).exceptionally(e -> {
LanguageServerPlugin.logError(e);
initializeFuture.completeExceptionally(e);
stop();
return null;
});
}

private CompletableFuture<InitializeResult> initServer(final URI rootURI) {
Expand Down Expand Up @@ -427,11 +448,13 @@ boolean isWrapperFor(LanguageServer server) {
return server == this.languageServer;
}

public synchronized void stop() {
final boolean alreadyStopping = this.stopping.getAndSet(true);
if (alreadyStopping) {
return;
public synchronized CompletableFuture<Void> stop() {
CompletableFuture<Void> result = new CompletableFuture<Void>();
CompletableFuture<Void> alreadyStopping = this.stopping.compareAndExchange(null, result);
if (alreadyStopping != null) {
return alreadyStopping;
}
try {
removeStopTimerTask();
if (this.initializeFuture != null) {
this.initializeFuture.cancel(true);
Expand All @@ -454,7 +477,7 @@ public synchronized void stop() {
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
} catch (Exception ex) {
LanguageServerPlugin.logError(ex);
LOG.error(ex.getLocalizedMessage(), ex);
}
}

Expand All @@ -469,10 +492,18 @@ public synchronized void stop() {
if (provider != null) {
provider.stop();
}
this.stopping.set(false);
};

CompletableFuture.runAsync(shutdownKillAndStopFutureAndProvider);
CompletableFuture.runAsync(shutdownKillAndStopFutureAndProvider, dispatcher).whenComplete((ignored, error) -> {
if (error != null) {
result.completeExceptionally(error);
} else {
result.complete(ignored);
}
if (!this.stopping.compareAndSet(result, null)) {
LOG.error("Unexpected concurrent stop", new IllegalStateException()); //$NON-NLS-1$
}
});

this.launcherFuture = null;
this.lspStreamProvider = null;
Expand All @@ -483,6 +514,10 @@ public synchronized void stop() {
this.languageServer = null;

FileBuffers.getTextFileBufferManager().removeFileBufferListener(fileBufferListener);
} catch (Exception e) {
result.completeExceptionally(e);
}
return result;
}

public @Nullable CompletableFuture<@NonNull LanguageServerWrapper> connect(IDocument document, @NonNull IFile file)
Expand Down Expand Up @@ -605,12 +640,14 @@ public CompletableFuture<Void> disconnect(URI uri) {
if (documentListener != null) {
documentListener.getDocument().removeDocumentListener(documentListener);
documentClosedFuture = documentListener.documentClosed();
} else {
documentClosedFuture = CompletableFuture.completedFuture(null);
}
if (this.connectedDocuments.isEmpty()) {
if (this.serverDefinition.lastDocumentDisconnectedTimeout != 0) {
startStopTimerTask();
} else {
stop();
documentClosedFuture = documentClosedFuture.thenCompose(ignored -> this.stop());
}
}
return documentClosedFuture;
Expand Down Expand Up @@ -1123,4 +1160,9 @@ private boolean isValid(WorkspaceFolder wsFolder) {

}

@Override
public String toString() {
return serverDefinition.id;
}

}

0 comments on commit f2967cb

Please sign in to comment.