Skip to content

Commit

Permalink
fix: improve startup and shutdown
Browse files Browse the repository at this point in the history 
In #1103, it was noted that
LanguageServerWrapperTest#testStopAndActivate causes OOM errors. On
investigation, it was discovered that there are multiple causes for
this.

1) The MockConnectionProviderMultiRootFolder used in the test created a
message processor thread in LSP4J on each call to start() but did not
shut them down.
2) The termination of the start/stop loop in the test was wrong: the
loop ran for as long as the VM running tests was alive.
3) The "already stopping" logic in LanguageServerWrapper#shutdown was
wrong: while the shutdown of a LanguageServerWrapper was processed,
further calls to shutdown were ignored.
4) There was a race between the initializationFuture of the
LanguageServerWrapper and the future used for shutdown.

With this commit, the test is rewritten to avoid crashes and to properly
test that calling stop / start repeatedly on LanguageServerWrapper does
not leave any connection providers running. The LanguageServerWrapper
class is refactored to make the test pass.
  • Loading branch information
@ava-fred
ava-fred committed Oct 2, 2024
1 parent 68f9534 commit 41e87e8
Show file tree
Hide file tree
Showing 3 changed files with 269 additions and 175 deletions.
84 changes: 56 additions & 28 deletions org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/LanguageServerWrapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
import static org.junit.Assert.assertTrue;

import java.util.Collection;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.TimeUnit;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
Expand All @@ -29,6 +30,7 @@
import org.eclipse.lsp4e.LanguageServerWrapper;
import org.eclipse.lsp4e.LanguageServiceAccessor;
import org.eclipse.lsp4e.test.utils.AbstractTestWithProject;
import org.eclipse.lsp4e.test.utils.MockConnectionProviderMultiRootFolders;
import org.eclipse.lsp4e.test.utils.TestUtils;
import org.eclipse.ui.IEditorPart;
import org.junit.Before;
Expand Down Expand Up @@ -73,44 +75,70 @@ public void testConnect() throws Exception {
* @see https://github.com/eclipse/lsp4e/pull/688
*/
@Test
public void testStopAndActive() throws CoreException, AssertionError, InterruptedException, ExecutionException {
if (System.getProperty("os.name").toLowerCase().startsWith("windows") || System.getenv("JENKINS_URL") != null) {
// FIXME temporarily disabling test on Windows and Jenkins because of https://github.com/eclipse/lsp4e/issues/1103
System.err.println("Temporarily skipping test execution. See https://github.com/eclipse/lsp4e/issues/1103");
return;
}
public void testStartStopAndActive() throws CoreException, AssertionError, InterruptedException, ExecutionException {
final int testCount= 100;

MockConnectionProviderMultiRootFolders.resetCounts();

IFile testFile1 = TestUtils.createFile(project, "shouldUseExtension.lsptWithMultiRoot", "");
IEditorPart editor1 = TestUtils.openEditor(testFile1);
@NonNull Collection<LanguageServerWrapper> wrappers = LanguageServiceAccessor.getLSWrappers(testFile1, request -> true);
assertEquals(1, wrappers.size());
LanguageServerWrapper wrapper = wrappers.iterator().next();
final var started = new CountDownLatch(1);

final int startingActiveThreads= ForkJoinPool.commonPool().getActiveThreadCount();

CompletableFuture<Void> startStop= CompletableFuture.runAsync(() -> {
for (int i= 0; i < testCount - 1; i++) {
wrapper.stop();
wrapper.start();
}
wrapper.stop();
});

CompletableFuture<Void> testActive= CompletableFuture.runAsync(() -> {
while (!startStop.isDone()) {
wrapper.isActive();
}
});

try {
var startStopJob = ForkJoinPool.commonPool().submit(() -> {
started.countDown();
while (!Thread.interrupted()) {
wrapper.stop();
wrapper.start();
}
});
startStop.get(30, TimeUnit.SECONDS);

try {
started.await();
for (int i = 0; i < 10000000; i++) {
// Should not throw
wrapper.isActive();
if (startStopJob.isDone()) {
throw new AssertionError("Job should run indefinitely");
}
}
} finally {
startStopJob.cancel(true);
if (!startStopJob.isCancelled()) {
startStopJob.get();
}
testActive.get(1, TimeUnit.SECONDS);
} catch (Exception e) {
throw new AssertionError("testActive terminated with exception");
}

} catch (Exception e) {
throw new AssertionError("test job terminated with exception");
//TODO improve diagnostics: check for timeout

} finally {
TestUtils.closeEditor(editor1, false);
}

// Give the various futures created time to execute. ForkJoinPool.commonPool.awaitQuiescence does not
// work here - other tests may not have cleaned up correctly.
long timeOut= System.currentTimeMillis() + 60_000;
do {
try {
Thread.sleep(1_000);
} catch (InterruptedException e) {
//ignore
}
} while (ForkJoinPool.commonPool().getActiveThreadCount() > startingActiveThreads && System.currentTimeMillis() < timeOut);

if (ForkJoinPool.commonPool().getActiveThreadCount() > startingActiveThreads) {
throw new AssertionError("timeout waiting for ForkJoinPool.commonPool to go quiet");
} else {

Integer cpStartCount= MockConnectionProviderMultiRootFolders.getStartCount();
Integer cpStopCount= MockConnectionProviderMultiRootFolders.getStopCount();

assertEquals("startCount == stopCount", cpStartCount, cpStopCount);
}
}

}
81 changes: 63 additions & 18 deletions ...e.lsp4e.test/src/org/eclipse/lsp4e/test/utils/MockConnectionProviderMultiRootFolders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.eclipse.lsp4e.server.StreamConnectionProvider;
import org.eclipse.lsp4e.tests.mock.MockLanguageServer;
Expand All @@ -30,32 +38,65 @@
import org.eclipse.lsp4j.launch.LSPLauncher;
import org.eclipse.lsp4j.services.LanguageClient;

import com.google.common.util.concurrent.ThreadFactoryBuilder;

public class MockConnectionProviderMultiRootFolders implements StreamConnectionProvider {
private static final Logger LOG = Logger.getLogger(MockConnectionProviderMultiRootFolders.class.getName());

static ExecutorService sharedExecutor = new ThreadPoolExecutor(0, Runtime.getRuntime().availableProcessors(), 0,
TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>(),
new ThreadFactoryBuilder().setNameFormat("mock-connection-provider-%d").build());

static private AtomicInteger startCount = new AtomicInteger(0);
static private AtomicInteger stopCount = new AtomicInteger(0);

private InputStream clientInputStream ;
static public void resetCounts() {
startCount.set(0);
stopCount.set(0);
}

static public int getStartCount() {
return startCount.get();
}

static public int getStopCount() {
return stopCount.get();
}

private InputStream clientInputStream;
private OutputStream clientOutputStream;
private InputStream errorStream;
private Collection<Closeable> streams = new ArrayList<>(4);
private Future<Void> launcherFuture;

@Override
public void start() throws IOException {
Pipe serverOutputToClientInput = Pipe.open();
Pipe clientOutputToServerInput = Pipe.open();
errorStream = new ByteArrayInputStream("Error output on console".getBytes(StandardCharsets.UTF_8));

InputStream serverInputStream = Channels.newInputStream(clientOutputToServerInput.source());
OutputStream serverOutputStream = Channels.newOutputStream(serverOutputToClientInput.sink());
Launcher<LanguageClient> launcher = LSPLauncher.createServerLauncher(MockLanguageServerMultiRootFolders.INSTANCE, serverInputStream,
serverOutputStream);
clientInputStream = Channels.newInputStream(serverOutputToClientInput.source());
clientOutputStream = Channels.newOutputStream(clientOutputToServerInput.sink());
launcher.startListening();
MockLanguageServer.INSTANCE.addRemoteProxy(launcher.getRemoteProxy());
streams.add(clientInputStream);
streams.add(clientOutputStream);
streams.add(serverInputStream);
streams.add(serverOutputStream);
streams.add(errorStream);
try {
Pipe serverOutputToClientInput = Pipe.open();
Pipe clientOutputToServerInput = Pipe.open();
errorStream = new ByteArrayInputStream("Error output on console".getBytes(StandardCharsets.UTF_8));

InputStream serverInputStream = Channels.newInputStream(clientOutputToServerInput.source());
OutputStream serverOutputStream = Channels.newOutputStream(serverOutputToClientInput.sink());

Launcher<LanguageClient> launcher = LSPLauncher.createServerLauncher(
MockLanguageServerMultiRootFolders.INSTANCE, serverInputStream, serverOutputStream, sharedExecutor,
(c) -> c);

clientInputStream = Channels.newInputStream(serverOutputToClientInput.source());
clientOutputStream = Channels.newOutputStream(clientOutputToServerInput.sink());
launcherFuture = launcher.startListening();
MockLanguageServer.INSTANCE.addRemoteProxy(launcher.getRemoteProxy());
streams.add(clientInputStream);
streams.add(clientOutputStream);
streams.add(serverInputStream);
streams.add(serverOutputStream);
streams.add(errorStream);

startCount.incrementAndGet();
} catch (Exception x) {
LOG.log(Level.SEVERE, "MockConnectionProvider#start", x);
}
}

@Override
Expand All @@ -75,5 +116,9 @@ public InputStream getErrorStream() {

@Override
public void stop() {
stopCount.incrementAndGet();
if (launcherFuture != null) {
launcherFuture.cancel(true);
}
}
}
Loading

0 comments on commit 41e87e8

Please sign in to comment.