Skip to content

Commit

Permalink
fix: Closing one module window doesn't shutdown the running LS instances
Browse files Browse the repository at this point in the history
Fixes #1069

Signed-off-by: azerr <[email protected]>
  • Loading branch information
angelozerr authored and fbricon committed Aug 2, 2023
1 parent 23f727d commit 7ffb092
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void projectOpened(@NotNull Project project) {
@Override
public void projectClosing(@NotNull Project project) {
LanguageServerLifecycleManager.getInstance(project).dispose();
LanguageServiceAccessor.getInstance(project).shutdownAllDispatchers();
LanguageServiceAccessor.getInstance(project).projectClosing(project);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
*******************************************************************************/
package com.redhat.devtools.intellij.lsp4ij;

import com.intellij.openapi.Disposable;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.vfs.VirtualFile;
import com.redhat.devtools.intellij.lsp4ij.operations.diagnostics.LSPDiagnosticsForServer;
Expand All @@ -35,7 +38,7 @@
*
* @author Angelo ZERR
*/
public class LSPVirtualFileWrapper {
public class LSPVirtualFileWrapper implements Disposable {

private static final Key<LSPVirtualFileWrapper> KEY = new Key<>(LSPVirtualFileWrapper.class.getName());

Expand All @@ -46,6 +49,10 @@ public class LSPVirtualFileWrapper {
LSPVirtualFileWrapper(VirtualFile file) {
this.file = file;
this.dataPerServer = new HashMap<>();
Module project = LSPIJUtils.getProject(file);
if (project != null) {
Disposer.register(project, this);
}
}

public VirtualFile getFile() {
Expand Down Expand Up @@ -129,7 +136,9 @@ private <T> Collection<T> getData(Function<LSPVirtualFileData, ? extends T> mapp

// ------------------------ Static accessor

@Override
public void dispose() {
file.putUserData(KEY, null);
this.dataPerServer.clear();
}

Expand Down Expand Up @@ -165,7 +174,6 @@ public static void dispose(VirtualFile file) {
LSPVirtualFileWrapper wrapper = file.getUserData(KEY);
if (wrapper != null) {
wrapper.dispose();
file.putUserData(KEY, null);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.intellij.AppTopics;
import com.intellij.ProjectTopics;
import com.intellij.lang.Language;
import com.intellij.openapi.Disposable;
import com.intellij.openapi.application.ApplicationInfo;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.editor.Document;
Expand All @@ -29,6 +30,7 @@
import com.intellij.openapi.module.Module;
import com.intellij.openapi.project.ModuleListener;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.messages.MessageBusConnection;
import com.redhat.devtools.intellij.lsp4ij.client.LanguageClientImpl;
Expand Down Expand Up @@ -60,12 +62,14 @@
/**
* Language server wrapper.
*/
public class LanguageServerWrapper {
public class LanguageServerWrapper implements Disposable {

private static final Logger LOGGER = LoggerFactory.getLogger(LanguageServerWrapper.class);//$NON-NLS-1$
private static final String CLIENT_NAME = "IntelliJ";
private static final int MAX_NUMBER_OF_RESTART_ATTEMPTS = 20; // TODO move this max value in settings

class Listener implements DocumentListener, FileDocumentManagerListener, FileEditorManagerListener {

@Override
public void documentChanged(@NotNull DocumentEvent event) {
URI uri = LSPIJUtils.toUri(event.getDocument());
Expand All @@ -86,7 +90,6 @@ public void beforeDocumentSaving(@NotNull Document document) {
}*/
}


@Override
public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile file) {
URI uri = LSPIJUtils.toUri(file);
Expand All @@ -95,7 +98,7 @@ public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile f
// Remove the cached file wrapper if needed
LSPVirtualFileWrapper.dispose(file);
// Disconnect the given file from all language servers
disconnect(uri);
disconnect(uri, isDisposed());
} catch (Exception e) {
LOGGER.warn("Error while disconnecting the file '" + uri + "' from all language servers", e);
}
Expand Down Expand Up @@ -131,6 +134,8 @@ public void fileClosed(@NotNull FileEditorManager source, @NotNull VirtualFile f

private ServerStatus serverStatus;

private boolean disposed;

private LanguageServerException serverError;

private Long currentProcessId;
Expand Down Expand Up @@ -179,6 +184,12 @@ private LanguageServerWrapper(@Nullable Module project, @Nonnull LanguageServers
this.listener = Executors
.newCachedThreadPool(new ThreadFactoryBuilder().setNameFormat(listenerThreadNameFormat).build());
udateStatus(ServerStatus.none);
if (project != null) {
// When project is disposed, we dispose the language server
// But the language server should be disposed before because when project is closing
// We do that to be sure that language server is disposed.
Disposer.register(project, this);
}
}

@Nonnull
Expand Down Expand Up @@ -469,6 +480,17 @@ public boolean isActive() {
return this.launcherFuture != null && !this.launcherFuture.isDone() && !this.launcherFuture.isCancelled();
}

@Override
public void dispose() {
this.disposed= true;
stop();
stopDispatcher();
}

public boolean isDisposed() {
return disposed;
}

/**
* Returns true if the language server is stopping and false otherwise.
*
Expand Down Expand Up @@ -504,48 +526,24 @@ public synchronized void stop(boolean alreadyStopping) {
this.serverCapabilities = null;
this.dynamicRegistrations.clear();

// We need to shutdown, kill and stop the process in a thread to avoid for instance
// stopping the new process created with a new start.
final Future<?> serverFuture = this.launcherFuture;
final StreamConnectionProvider provider = this.lspStreamProvider;
final LanguageServer languageServerInstance = this.languageServer;

Runnable shutdownKillAndStopFutureAndProvider = () -> {
if (languageServerInstance != null && provider != null && provider.isAlive()) {
// The LSP language server instance and the process which starts the language server is alive. Process
// - shutdown
// - exit

// shutdown the language server
try {
shutdownLanguageServerInstance(languageServerInstance);
} catch (Exception ex) {
getLanguageServerLifecycleManager().onError(this, ex);
}

// exit the language server
// Consume language server exit() before cancelling launcher future (serverFuture.cancel())
// to avoid having error like "The pipe is being closed".
try {
exitLanguageServerInstance(languageServerInstance);
} catch (Exception ex) {
getLanguageServerLifecycleManager().onError(this, ex);
}
}

if (serverFuture != null) {
serverFuture.cancel(true);
}

if (provider != null) {
provider.stop();
}
this.stopping.set(false);
udateStatus(ServerStatus.stopped);
getLanguageServerLifecycleManager().onStatusChanged(this);
};

CompletableFuture.runAsync(shutdownKillAndStopFutureAndProvider);
if (isDisposed()) {
// When project is closing we shutdown everything in synch mode
shutdownAll(languageServer, lspStreamProvider, launcherFuture);
} else {
// We need to shutdown, kill and stop the process in a thread to avoid for instance
// stopping the new process created with a new start.
final Future<?> serverFuture = this.launcherFuture;
final StreamConnectionProvider provider = this.lspStreamProvider;
final LanguageServer languageServerInstance = this.languageServer;

Runnable shutdownKillAndStopFutureAndProvider = () -> {
shutdownAll(languageServerInstance, provider, serverFuture);
this.stopping.set(false);
udateStatus(ServerStatus.stopped);
getLanguageServerLifecycleManager().onStatusChanged(this);
};
CompletableFuture.runAsync(shutdownKillAndStopFutureAndProvider);
}
} finally {
this.launcherFuture = null;
this.lspStreamProvider = null;
Expand All @@ -563,6 +561,38 @@ public synchronized void stop(boolean alreadyStopping) {
}
}

private void shutdownAll(LanguageServer languageServerInstance, StreamConnectionProvider provider, Future<?> serverFuture) {
if (languageServerInstance != null && provider != null && provider.isAlive()) {
// The LSP language server instance and the process which starts the language server is alive. Process
// - shutdown
// - exit

// shutdown the language server
try {
shutdownLanguageServerInstance(languageServerInstance);
} catch (Exception ex) {
getLanguageServerLifecycleManager().onError(this, ex);
}

// exit the language server
// Consume language server exit() before cancelling launcher future (serverFuture.cancel())
// to avoid having error like "The pipe is being closed".
try {
exitLanguageServerInstance(languageServerInstance);
} catch (Exception ex) {
getLanguageServerLifecycleManager().onError(this, ex);
}
}

if (serverFuture != null) {
serverFuture.cancel(true);
}

if (provider != null) {
provider.stop();
}
}

private void shutdownLanguageServerInstance(LanguageServer languageServerInstance) throws Exception {
CompletableFuture<Object> shutdown = languageServerInstance.shutdown();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,15 @@ void shutdownAllDispatchers() {
startedServers.forEach(LanguageServerWrapper::stopDispatcher);
}

public void projectClosing(Project project) {
// On project closing, we dispose all language servers
startedServers.forEach(ls -> {
if (project.equals(ls.getProject())) {
ls.dispose();
}
});
}

/**
* A bean storing association of a Document/File with a language server.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@
package com.redhat.devtools.intellij.lsp4ij.client;

import com.intellij.openapi.module.Module;
import com.intellij.openapi.progress.*;
import com.intellij.openapi.progress.ProgressIndicator;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.roots.ModuleRootManager;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.vfs.VfsUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils;
Expand All @@ -29,7 +28,6 @@ public class IndexAwareLanguageClient extends LanguageClientImpl {

public IndexAwareLanguageClient(Project project) {
super(project);
Disposer.register(project, this);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.module.Module;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Disposer;
import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils;
import com.redhat.devtools.intellij.lsp4ij.LanguageServerWrapper;
import com.redhat.devtools.intellij.lsp4ij.ServerMessageHandler;
Expand All @@ -30,6 +31,7 @@ public class LanguageClientImpl implements LanguageClient, Disposable {

public LanguageClientImpl(Project project) {
this.project = project;
Disposer.register(project, this);
}

public Project getProject() {
Expand Down

0 comments on commit 7ffb092

Please sign in to comment.