Improve robustness against wrong call order (#165)

- in case stopServer is called before a startServer - added start and stop in tests - thanks to previous test detected and fixed a remaining Java thread when stop server called Signed-off-by: Aurélien Pupier <[email protected]>
camel-tooling · Oct 19, 2018 · 18eaca9 · 18eaca9
1 parent 5a8a013
commit 18eaca9
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 4 deletions.
diff --git a/src/main/java/com/github/cameltooling/lsp/internal/AbstractLanguageServer.java b/src/main/java/com/github/cameltooling/lsp/internal/AbstractLanguageServer.java
@@ -33,15 +33,17 @@ private final class CamelServerRunnable implements Runnable {
 		@Override
 		public void run() {
 			LOGGER.info("Starting Camel Language Server...");
-			while (!shutdown && parentProcessStillRunning()) {
+			while (!shutdown && parentProcessStillRunning() && !Thread.currentThread().isInterrupted()) {
 				try {
 					Thread.sleep(1000);
 				} catch (InterruptedException e) {
 					LOGGER.error(e.getMessage(), e);
 					Thread.currentThread().interrupt();
 				}
 			}
-			LOGGER.info("Camel Language Server - Client vanished...");
+			if (!Thread.currentThread().isInterrupted()) {
+				LOGGER.info("Camel Language Server - Client vanished...");
+			}
 		}
 	}
 
@@ -106,7 +108,11 @@ protected boolean parentProcessStillRunning() {
 	 */
 	public void stopServer() {
 		LOGGER.info("Stopping language server");
-		runner.interrupt();
+		if (runner != null) {
+			runner.interrupt();
+		} else {
+			LOGGER.info("Request to stop the server has been received but it wasn't started.");
+		}
 	}
 
 	/**

diff --git a/src/test/java/com/github/cameltooling/lsp/internal/AbstractCamelLanguageServerTest.java b/src/test/java/com/github/cameltooling/lsp/internal/AbstractCamelLanguageServerTest.java
@@ -1,3 +1,19 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package com.github.cameltooling.lsp.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -38,6 +54,7 @@
 import org.eclipse.lsp4j.jsonrpc.messages.Either;
 import org.eclipse.lsp4j.services.LanguageClient;
 import org.eclipse.lsp4j.services.TextDocumentService;
+import org.junit.After;
 
 public abstract class AbstractCamelLanguageServerTest {
 
@@ -47,10 +64,18 @@ public abstract class AbstractCamelLanguageServerTest {
 	protected static final String DUMMY_URI = "dummyUri";
 	private String extensionUsed;
 	protected PublishDiagnosticsParams lastPublishedDiagnostics;
+	private CamelLanguageServer camelLanguageServer;
 
 	public AbstractCamelLanguageServerTest() {
 		super();
 	}
+
+	@After
+	public void tearDown() {
+		if (camelLanguageServer != null) {
+			camelLanguageServer.stopServer();
+		}
+	}
 
 	protected CompletionItem createExpectedAhcCompletionItem(int lineStart, int characterStart, int lineEnd, int characterEnd) {
 		CompletionItem expectedAhcCompletioncompletionItem = new CompletionItem("ahc:httpUri");
@@ -94,8 +119,9 @@ protected CamelLanguageServer initializeLanguageServer(String text, String suffi
 		InitializeParams params = new InitializeParams();
 		params.setProcessId(new Random().nextInt());
 		params.setRootUri(getTestResource("/workspace/").toURI().toString());
-		CamelLanguageServer camelLanguageServer = new CamelLanguageServer();
+		camelLanguageServer = new CamelLanguageServer();
 		camelLanguageServer.connect(new DummyLanguageClient());
+		camelLanguageServer.startServer();
 		CompletableFuture<InitializeResult> initialize = camelLanguageServer.initialize(params);
 
 		assertThat(initialize).isCompleted();

diff --git a/src/test/java/com/github/cameltooling/lsp/internal/CamelLanguageServerRobustnesstest.java b/src/test/java/com/github/cameltooling/lsp/internal/CamelLanguageServerRobustnesstest.java
@@ -0,0 +1,28 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.cameltooling.lsp.internal;
+
+import org.junit.Test;
+
+public class CamelLanguageServerRobustnesstest {
+
+	@Test
+	public void testStopWithoutErrorIfStopCalledWithoutConnection() throws Exception {
+		new CamelLanguageServer().stopServer();
+	}
+
+}