diff options
author | Torbjörn SVENSSON | 2021-10-28 18:55:24 +0000 |
---|---|---|
committer | Jonah Graham | 2021-11-04 14:43:19 +0000 |
commit | acaba8e57e6c6e3557496d6440a23685f4dcdade (patch) | |
tree | 44466f825d5678c00270f47f6a345e05b651f2e6 | |
parent | de0175a37b1e0b092b42ce79afd270e5c6c60a13 (diff) | |
download | eclipse.platform.debug-acaba8e57e6c6e3557496d6440a23685f4dcdade.tar.gz eclipse.platform.debug-acaba8e57e6c6e3557496d6440a23685f4dcdade.tar.xz eclipse.platform.debug-acaba8e57e6c6e3557496d6440a23685f4dcdade.zip |
Bug 576945: StreamsProxy needs to be closed after app exitI20211104-1800
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>
3 files changed, 130 insertions, 29 deletions
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 98122a8d6..35f17f8c6 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 @@ public class RuntimeProcess extends PlatformObject implements IProcess { @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); + 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 + } + } - // 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; + 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(); } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); } // 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 21f388053..992a0c3a6 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 @@ package org.eclipse.debug.tests.console; 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 @@ public class MockProcess extends Process { 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 @@ public class MockProcess extends Process { } /** + * 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 @@ public class MockProcess extends Process { } } } - handle.ifPresent(MockProcessHandle::setTerminated); + setTerminated(); return exitCode; } @@ -213,7 +267,7 @@ public class MockProcess extends Process { } } if (isTerminated()) { - handle.ifPresent(MockProcessHandle::setTerminated); + setTerminated(); } return isTerminated(); } @@ -239,11 +293,12 @@ public class MockProcess extends Process { * 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 @@ public class MockProcess extends Process { } 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 bbcca06ea..c29a24984 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.assertFalse; 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 @@ public class RuntimeProcessTests extends AbstractDebugTest { 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()))); + } + + } |