From c7998e17546f2beb9af0d53585484d74eb4ad716 Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Wed, 9 Aug 2017 17:49:45 +0300 Subject: [PATCH] ensure external javac process is terminated in tests --- .../compiler/CompilerManagerImpl.java | 10 ++++++ .../testFramework/CompilerTester.java | 10 +++++- .../jps/javac/ExternalJavacManager.java | 31 ++++++++++++++----- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/java/compiler/impl/src/com/intellij/compiler/CompilerManagerImpl.java b/java/compiler/impl/src/com/intellij/compiler/CompilerManagerImpl.java index c80a3be30c00..fdcf95c68e90 100644 --- a/java/compiler/impl/src/com/intellij/compiler/CompilerManagerImpl.java +++ b/java/compiler/impl/src/com/intellij/compiler/CompilerManagerImpl.java @@ -62,6 +62,7 @@ import java.lang.reflect.Array; import java.net.URI; import java.util.*; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; public class CompilerManagerImpl extends CompilerManager { private static final Logger LOG = Logger.getInstance("#com.intellij.compiler.CompilerManagerImpl"); @@ -116,6 +117,15 @@ public class CompilerManagerImpl extends CompilerManager { }); } + // returns true if all javacs terminated + public boolean waitForExternalJavacToTerminate(long time, @NotNull TimeUnit unit) { + ExternalJavacManager externalJavacManager = myExternalJavacManager; + if (externalJavacManager != null) { + if (!externalJavacManager.waitForAllProcessHandlers(time, unit)) return false; + } + return true; + } + public Semaphore getCompilationSemaphore() { return myCompilationSemaphore; } diff --git a/java/testFramework/src/com/intellij/testFramework/CompilerTester.java b/java/testFramework/src/com/intellij/testFramework/CompilerTester.java index feb0142e5faf..81d0c9cc74ae 100644 --- a/java/testFramework/src/com/intellij/testFramework/CompilerTester.java +++ b/java/testFramework/src/com/intellij/testFramework/CompilerTester.java @@ -15,7 +15,9 @@ */ package com.intellij.testFramework; +import com.intellij.compiler.CompilerManagerImpl; import com.intellij.compiler.CompilerTestUtil; +import com.intellij.diagnostic.ThreadDumper; import com.intellij.openapi.application.Result; import com.intellij.openapi.application.WriteAction; import com.intellij.openapi.command.WriteCommandAction; @@ -53,6 +55,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.TimeUnit; /** * @author peter @@ -204,6 +207,11 @@ public class CompilerTester { } callback.throwException(); + + if (!((CompilerManagerImpl)CompilerManager.getInstance(getProject())).waitForExternalJavacToTerminate(1, TimeUnit.MINUTES)) { + throw new RuntimeException("External javac thread is still running. Thread dump:" + ThreadDumper.dumpThreadsToString()); + } + return callback.getMessages(); } @@ -219,7 +227,7 @@ public class CompilerTester { private Throwable myError; private final List myMessages = new ArrayList<>(); - public ErrorReportingCallback(Semaphore semaphore) { + ErrorReportingCallback(Semaphore semaphore) { mySemaphore = semaphore; } diff --git a/jps/jps-builders/src/org/jetbrains/jps/javac/ExternalJavacManager.java b/jps/jps-builders/src/org/jetbrains/jps/javac/ExternalJavacManager.java index 21b02e07c7a3..c61445ac7806 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/javac/ExternalJavacManager.java +++ b/jps/jps-builders/src/org/jetbrains/jps/javac/ExternalJavacManager.java @@ -15,10 +15,7 @@ */ package org.jetbrains.jps.javac; -import com.intellij.execution.process.BaseOSProcessHandler; -import com.intellij.execution.process.ProcessAdapter; -import com.intellij.execution.process.ProcessEvent; -import com.intellij.execution.process.ProcessOutputTypes; +import com.intellij.execution.process.*; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.SystemInfo; @@ -26,6 +23,7 @@ import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.util.text.StringUtil; import com.intellij.util.ConcurrencyUtil; import com.intellij.util.concurrency.Semaphore; +import com.intellij.util.containers.ContainerUtil; import io.netty.bootstrap.ServerBootstrap; import io.netty.channel.*; import io.netty.channel.group.ChannelGroup; @@ -72,6 +70,7 @@ public class ExternalJavacManager { private final ChannelRegistrar myChannelRegistrar; private final Map myMessageHandlers = new HashMap<>(); private int myListenPort = DEFAULT_SERVER_PORT; + private final Set myRunningHandlers = ContainerUtil.newConcurrentSet(); public ExternalJavacManager(@NotNull final File workingDir) { myWorkingDir = workingDir; @@ -132,8 +131,15 @@ public class ExternalJavacManager { final ExternalJavacProcessHandler processHandler = launchExternalJavacProcess( uuid, javaHome, heapSize, myListenPort, myWorkingDir, vmOptions, compilingTool ); + myRunningHandlers.add(processHandler); processHandler.addProcessListener(new ProcessAdapter() { - public void onTextAvailable(ProcessEvent event, Key outputType) { + @Override + public void processTerminated(@NotNull ProcessEvent event) { + myRunningHandlers.remove(processHandler); + } + + @Override + public void onTextAvailable(@NotNull ProcessEvent event, @NotNull Key outputType) { final String text = event.getText(); if (!StringUtil.isEmptyOrSpaces(text)) { String prefix = null; @@ -161,7 +167,7 @@ public class ExternalJavacManager { processDescriptor.cancelBuild(); } } - + System.out.println(processHandler.isProcessTerminated()); return rh.isTerminatedSuccessfully(); } catch (Throwable e) { @@ -174,6 +180,16 @@ public class ExternalJavacManager { return false; } + // returns true if all process handlers terminated + public boolean waitForAllProcessHandlers(long time, TimeUnit unit) { + for (ProcessHandler handler : myRunningHandlers) { + if (!handler.waitFor(unit.toMillis(time))) { + return false; + } + } + return true; + } + private void unregisterMessageHandler(UUID uuid) { final JavacProcessDescriptor descriptor; synchronized (myMessageHandlers) { @@ -296,12 +312,13 @@ public class ExternalJavacManager { super(process, commandLine, null); addProcessListener(new ProcessAdapter() { @Override - public void processTerminated(ProcessEvent event) { + public void processTerminated(@NotNull ProcessEvent event) { myExitCode = event.getExitCode(); } }); } + @Override @NotNull public Integer getExitCode() { return myExitCode;