Bug 576945: StreamsProxy needs to be closed after app exit Capture and forward all output from application by closing the StreamsProxy after the graceful exit timeout. Contributed by STMicroelectronics Change-Id: I139e414ee0b617085cbe66df3d788e73f1b50e78 Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.debug/+/187134 Tested-by: Platform Bot <platform-bot@eclipse.org> Reviewed-by: Jonah Graham <jonah@kichwacoders.com>
diff --git a/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java b/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java index 98122a8..35f17f8 100644 --- a/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java +++ b/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java
@@ -216,37 +216,41 @@ @Override public void terminate() throws DebugException { if (!isTerminated()) { - if (fStreamsProxy instanceof StreamsProxy) { - ((StreamsProxy) fStreamsProxy).kill(); - } - Process process = getSystemProcess(); - if (process == null) { - return; - } - - List<ProcessHandle> descendants = Collections.emptyList(); - if (fTerminateDescendants) { - try { // List of descendants of process is only a snapshot! - descendants = process.descendants().collect(Collectors.toList()); - } catch (UnsupportedOperationException e) { - // JVM may not support toHandle() -> assume no descendants + try { + Process process = getSystemProcess(); + if (process == null) { + return; } - } - process.destroy(); - descendants.forEach(ProcessHandle::destroy); - - // await termination of process and descendants - try { // (in total don't wait longer than TERMINATION_TIMEOUT) - long waitStart = System.currentTimeMillis(); - if (process.waitFor(TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) { - fExitValue = process.exitValue(); - if (waitFor(descendants, waitStart)) { - return; + List<ProcessHandle> descendants = Collections.emptyList(); + if (fTerminateDescendants) { + try { // List of descendants of process is only a snapshot! + descendants = process.descendants().collect(Collectors.toList()); + } catch (UnsupportedOperationException e) { + // JVM may not support toHandle() -> assume no + // descendants } } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + + process.destroy(); + descendants.forEach(ProcessHandle::destroy); + + // await termination of process and descendants + try { // (in total don't wait longer than TERMINATION_TIMEOUT) + long waitStart = System.currentTimeMillis(); + if (process.waitFor(TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) { + fExitValue = process.exitValue(); + if (waitFor(descendants, waitStart)) { + return; + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } finally { + if (fStreamsProxy instanceof StreamsProxy) { + ((StreamsProxy) fStreamsProxy).kill(); + } } // clean-up
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java index 21f3880..992a0c3 100644 --- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java +++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcess.java
@@ -15,6 +15,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.Map; @@ -73,6 +74,31 @@ private int terminationDelay = 0; /** + * For mode used by constructor {@link MockProcess#MockProcess()} this + * indicates via stdout what the state of the process is. + */ + public static enum ProcessState { + RUNNING('R'), DESTROYING('D'), + /** + * Last read is special as it should only be returned once to ensure + * that we get the last character on the stream. + */ + LASTREAD('L'), TERMINATED(-1); + + private int code; + + ProcessState(int c) { + this.code = c; + } + + public int getCode() { + return code; + } + } + + private volatile ProcessState processState = ProcessState.RUNNING; + + /** * Create new silent mockup process which runs for a given amount of time. * Does not read input or produce any output. * @@ -141,6 +167,34 @@ } /** + * Create a new mock process that the stdout stream will indicate status of + * the process. The codes are defined by {@link ProcessState#getCode()} + */ + public MockProcess() { + super(); + this.stderr = new ByteArrayInputStream(new byte[0]); + this.endTime = RUN_FOREVER; + this.stdout = new InputStream() { + @Override + public int read() throws IOException { + if (processState == ProcessState.LASTREAD) { + + // Uncomment this sleep and the test will fail because + // RuntimeProcess.terminate does not wait until + // the monitor threads complete. + // try { + // Thread.sleep(1000); + // } catch (InterruptedException e) { + // } + processState = ProcessState.TERMINATED; + return ProcessState.LASTREAD.getCode(); + } + return processState.getCode(); + } + }; + } + + /** * Get bytes received through stdin since last invocation of this method. * <p> * Not thread safe. It may miss some input if new content is written while @@ -194,7 +248,7 @@ } } } - handle.ifPresent(MockProcessHandle::setTerminated); + setTerminated(); return exitCode; } @@ -213,7 +267,7 @@ } } if (isTerminated()) { - handle.ifPresent(MockProcessHandle::setTerminated); + setTerminated(); } return isTerminated(); } @@ -239,11 +293,12 @@ * and before the mockup process goes in terminated state */ public void destroy(int delay) { + processState = ProcessState.DESTROYING; synchronized (waitForTerminationLock) { endTime = System.currentTimeMillis() + delay; waitForTerminationLock.notifyAll(); if (delay <= 0) { - handle.ifPresent(MockProcessHandle::setTerminated); + setTerminated(); } } } @@ -333,4 +388,16 @@ } return (RuntimeProcess) DebugPlugin.newProcess(new Launch(launchConfiguration, ILaunchManager.RUN_MODE, null), this, name); } + + /** + * Move state machines to terminated. + */ + private void setTerminated() { + synchronized (this) { + if (processState != ProcessState.TERMINATED) { + processState = ProcessState.LASTREAD; + } + } + handle.ifPresent(MockProcessHandle::setTerminated); + } }
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/RuntimeProcessTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/RuntimeProcessTests.java index bbcca06..c29a249 100644 --- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/RuntimeProcessTests.java +++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/RuntimeProcessTests.java
@@ -21,17 +21,22 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import java.io.InputStream; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import org.eclipse.core.runtime.IStatus; import org.eclipse.debug.core.DebugEvent; import org.eclipse.debug.core.DebugException; import org.eclipse.debug.core.DebugPlugin; +import org.eclipse.debug.core.model.IProcess; import org.eclipse.debug.core.model.RuntimeProcess; import org.eclipse.debug.internal.core.DebugCoreMessages; import org.eclipse.debug.tests.AbstractDebugTest; import org.eclipse.debug.tests.TestUtil; +import org.eclipse.debug.tests.sourcelookup.TestLaunch; import org.junit.Test; public class RuntimeProcessTests extends AbstractDebugTest { @@ -206,4 +211,29 @@ DebugException timeoutException = assertThrows(DebugException.class, runtimeProcess::terminate); assertThat(timeoutException.getMessage(), is(DebugCoreMessages.RuntimeProcess_terminate_failed)); } + + @Test + public void testOutputAfterDestroy() throws Exception { + MockProcess proc = new MockProcess(); + + IProcess iProc = new RuntimeProcess(new TestLaunch(), proc, "foo", Collections.emptyMap()); + iProc.terminate(); + + String str = iProc.getStreamsProxy().getOutputStreamMonitor().getContents(); + TestUtil.log(IStatus.INFO, name.getMethodName(), "Stream result: "); + for (int i = 0; i < str.length(); i += 100) { + TestUtil.log(IStatus.INFO, name.getMethodName(), str.substring(i, Math.min(i + 100, str.length()))); + } + TestUtil.log(IStatus.INFO, name.getMethodName(), "Stream done."); + // Make sure that the inputstream (process's stdout) has been fully read + // and is at EOF + @SuppressWarnings("resource") + InputStream inputStream = proc.getInputStream(); + assertEquals(-1, inputStream.read()); + // Make sure that the last character in the stream makes it through to + // the monitor + assertTrue(str.endsWith(String.valueOf((char) MockProcess.ProcessState.LASTREAD.getCode()))); + } + + }