From 98f5de93ba1c7416d7c341d16ea00a309fe3fe57 Mon Sep 17 00:00:00 2001
From: Vasili Gulevich <vasili.gulevich@xored.com>
Date: Wed, 7 Jun 2023 21:11:18 +0400
Subject: [PATCH] Revert wrapper cleanup on shutdown (#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.
---
 .../lsp4e/LanguageServerWrapperTest.java      |  1 -
 .../eclipse/lsp4e/LanguageServerPlugin.java   |  2 +-
 .../lsp4e/LanguageServiceAccessor.java        | 27 +++++++------------
 3 files changed, 11 insertions(+), 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
index ee1838395..311d3d301 100644
--- 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
index 2cdce59da..d7df28137 100644
--- 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
index 964c161e7..07281e40d 100644
--- 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);
+	}
 }