Revert wrapper cleanup on shutdown (eclipse#681)

The cleanup on shutdown is special - it intentionally does not remove wrapper registration to prevent reinitialization of wrappers by client code. This way, situation when a wrapper is active after plugin stop is prevented and orderly shutdown is possible.
basilevs · Jun 7, 2023 · 98f5de9 · 98f5de9
1 parent b9c8949
commit 98f5de9
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 19 deletions.
diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/LanguageServerWrapperTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/LanguageServerWrapperTest.java
@@ -103,7 +103,6 @@ public void close() throws SecurityException {
 			waitForAndAssertCondition(2_000, () -> MockLanguageServerMultiRootFolders.INSTANCE.isRunning());
 			assertTrue(wrapper.isConnectedTo(testFile1.getLocationURI()));
 			logMessages.clear();
-			LanguageServiceAccessor.clearStartedServers();
 			wrapper.stopDispatcher();
 			waitForAndAssertCondition(2_000, () -> !MockLanguageServerMultiRootFolders.INSTANCE.isRunning());
 			Assert.assertEquals(Collections.emptyList(), logMessages);

diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerPlugin.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServerPlugin.java
@@ -40,7 +40,7 @@ public void start(BundleContext context) throws Exception {
 	@Override
 	public void stop(BundleContext context) throws Exception {
 		plugin = null;
-		LanguageServiceAccessor.clearStartedServers();
+		LanguageServiceAccessor.shutdownAllDispatchers();
 		super.stop(context);
 	}
 

diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServiceAccessor.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/LanguageServiceAccessor.java
@@ -36,10 +36,8 @@
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
-import org.eclipse.core.runtime.ILog;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Path;
-import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.content.IContentDescription;
 import org.eclipse.core.runtime.content.IContentType;
 import org.eclipse.jdt.annotation.NonNull;
@@ -65,28 +63,19 @@ private LanguageServiceAccessor() {
 		// this class shouldn't be instantiated
 	}
 
-	private static final String FAILED_TO_STOP_SERVER = "Failed to stop language server %s"; // //$NON-NLS-1$
-
-
 	private static final Set<@NonNull LanguageServerWrapper> startedServers = new CopyOnWriteArraySet<>();
 	private static final Map<StreamConnectionProvider, LanguageServerDefinition> providersToLSDefinitions = new HashMap<>();
-	private static final ILog LOG = Platform.getLog(LanguageServiceAccessor.class);
-
 
 	/**
 	 * This is meant for test code to clear state that might have leaked from other
-	 * tests. It isn't meant to be used in production code, except for Eclipse shutdown.
+	 * tests. It isn't meant to be used in production code.
 	 */
 	public static void clearStartedServers() {
-		startedServers.removeIf((wrapper) -> {
-			try {
-				wrapper.stopDispatcher();
-			} catch (Exception e) {
-				LOG.error(String.format(FAILED_TO_STOP_SERVER, wrapper), e);
-			}
-				return true;
-			}
- 		);
+		startedServers.removeIf(server -> {
+			server.stop();
+			server.stopDispatcher();
+			return true;
+		});
 	}
 
 	/**
@@ -611,5 +600,9 @@ protected static LanguageServerDefinition getLSDefinition(@NonNull StreamConnect
 		}
 		return CompletableFuture.completedFuture(Collections.emptyList());
 	}
+
+	static void shutdownAllDispatchers() {
+		startedServers.forEach(LanguageServerWrapper::stopDispatcher);
+	}
 }