Bug 567345 - Terminate descendants of RuntimeProcessY20201030-1200Y20201029-1200Y20201028-1200I20201029-1800I20201028-1800I20201028-0800I20201028-0610I20201028-0130I20201028-0020I20201027-2350I20201027-1800

The descendants (sub-processes and their recursive sub-processes) of the
java.lang.Process wrapped by a RuntimeProcess are destroyed too.

Change-Id: Ibabe73e34543aa1ff0be7361e63c1bab52a4f538
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>

4 files changed, 202 insertions, 5 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 6cf829bca..51186f956 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
@@ -13,14 +13,16 @@
  *******************************************************************************/
 package org.eclipse.debug.core.model;
 
-
 import java.nio.charset.Charset;
 import java.nio.charset.IllegalCharsetNameException;
 import java.nio.charset.UnsupportedCharsetException;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.PlatformObject;
@@ -34,7 +36,6 @@ import org.eclipse.debug.internal.core.DebugCoreMessages;
 import org.eclipse.debug.internal.core.NullStreamsProxy;
 import org.eclipse.debug.internal.core.StreamsProxy;
 
-
 /**
  * Standard implementation of an <code>IProcess</code> that wrappers a system
  * process (<code>java.lang.Process</code>).
@@ -203,11 +204,21 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	public void terminate() throws DebugException {
 		if (!isTerminated()) {
 			if (fStreamsProxy instanceof StreamsProxy) {
-				((StreamsProxy)fStreamsProxy).kill();
+				((StreamsProxy) fStreamsProxy).kill();
 			}
 			Process process = getSystemProcess();
 			if (process != null) {
+
+				List<ProcessHandle> descendants; // only a snapshot!
+				try {
+					descendants = process.descendants().collect(Collectors.toList());
+				} catch (UnsupportedOperationException e) {
+					// JVM may not support toHandle() -> assume no descendants
+					descendants = Collections.emptyList();
+				}
+
 				process.destroy();
+				descendants.forEach(ProcessHandle::destroy);
 			}
 			int attempts = 0;
 			while (attempts < MAX_WAIT_FOR_DEATH_ATTEMPTS) {
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 0c5667235..bac6a22d0 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
@@ -18,9 +18,9 @@ import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
-
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunchConfigurationType;
@@ -66,6 +66,9 @@ public class MockProcess extends Process {
 	/** The simulated exit code. */
 	private int exitCode = 0;
 
+	/** The child/sub mock-processes of this mock-process. */
+	private Optional<MockProcessHandle> handle = Optional.of(new MockProcessHandle(this));
+
 	/**
 	 * Create new silent mockup process which runs for a given amount of time.
 	 * Does not read input or produce any output.
@@ -166,6 +169,15 @@ public class MockProcess extends Process {
 	}
 
 	@Override
+	public ProcessHandle toHandle() {
+		if (handle.isPresent()) {
+			return handle.get();
+		}
+		// let super implementation throw the UnsupportedOperationException
+		return super.toHandle();
+	}
+
+	@Override
 	public int waitFor() throws InterruptedException {
 		synchronized (waitForTerminationLock) {
 			while (!isTerminated()) {
@@ -245,6 +257,16 @@ public class MockProcess extends Process {
 	}
 
 	/**
+	 * Set the {@link ProcessHandle} of the process. A null value indices that
+	 * this process does not support {@link Process#toHandle()}.
+	 *
+	 * @param handle new process handle
+	 */
+	public void setHandle(MockProcessHandle handle) {
+		this.handle = Optional.ofNullable(handle);
+	}
+
+	/**
 	 * Create a {@link RuntimeProcess} which wraps this {@link MockProcess}.
 	 * <p>
 	 * Note: the process will only be connected to a minimal dummy launch
diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcessHandle.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcessHandle.java
new file mode 100644
index 000000000..50841a660
--- /dev/null
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/MockProcessHandle.java
@@ -0,0 +1,109 @@
+/*******************************************************************************
+ * Copyright (c) 2020 Hannes Wellmann and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Hannes Wellmann - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.debug.tests.console;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+/**
+ * A mockup ProcessHandle which works in conjunction with {@link MockProcess}.
+ */
+public class MockProcessHandle implements ProcessHandle {
+
+	private final MockProcess process;
+	private final Collection<ProcessHandle> children;
+
+	/**
+	 * Create new mockup process handle for a process without children.
+	 *
+	 * @param process the process of this handle
+	 */
+	public MockProcessHandle(MockProcess process) {
+		this(process, Collections.emptyList());
+	}
+
+	/**
+	 * Create new mockup process handle for a process with the given children.
+	 *
+	 * @param process the process of this handle
+	 * @param children the child-processes of the given process
+	 */
+	public MockProcessHandle(MockProcess process, Collection<Process> children) {
+		this.process = process;
+		this.children = children.stream().map(Process::toHandle).collect(Collectors.toUnmodifiableList());
+	}
+
+	@Override
+	public Stream<ProcessHandle> children() {
+		return this.children.stream();
+	}
+
+	@Override
+	public Stream<ProcessHandle> descendants() {
+		return Stream.concat(children(), children().flatMap(ProcessHandle::descendants));
+	}
+
+	@Override
+	public boolean supportsNormalTermination() {
+		return true;
+	}
+
+	@Override
+	public boolean destroy() {
+		process.destroy();
+		return true;
+	}
+
+	@Override
+	public boolean destroyForcibly() {
+		return destroy();
+	}
+
+	@Override
+	public boolean isAlive() {
+		return process.isAlive();
+	}
+
+	@Override
+	public int compareTo(ProcessHandle other) {
+		return Long.compare(pid(), ((MockProcessHandle) other).pid());
+	}
+
+	// not yet implemented methods
+
+	@Override
+	public long pid() {
+		throw new UnsupportedOperationException("Not yet implemented");
+	}
+
+	@Override
+	public Optional<ProcessHandle> parent() {
+		throw new UnsupportedOperationException("Not yet implemented");
+	}
+
+	@Override
+	public Info info() {
+		throw new UnsupportedOperationException("Not yet implemented");
+	}
+
+	@Override
+	public CompletableFuture<ProcessHandle> onExit() {
+		throw new UnsupportedOperationException("Not yet implemented");
+	}
+}
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 0134dc349..7dbdd2215 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
@@ -15,8 +15,10 @@ package org.eclipse.debug.tests.console;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.eclipse.debug.core.DebugEvent;
@@ -78,11 +80,64 @@ public class RuntimeProcessTests extends AbstractDebugTest {
 
 		mockProcess.setExitValue(1);
 		runtimeProcess.terminate();
-		assertFalse("RuntimeProcess failed to terminated wrapped process.", mockProcess.isAlive());
+		assertFalse("RuntimeProcess failed to terminate wrapped process.", mockProcess.isAlive());
 
 		TestUtil.waitWhile(p -> !p.isTerminated(), runtimeProcess, 1000, p -> "RuntimePocess not terminated.");
 		TestUtil.waitForJobs(name.getMethodName(), 25, 500);
 		assertEquals("Wrong number of terminate events.", 1, processTerminateEvents.get());
 		assertEquals("RuntimeProcess reported wrong exit code.", 1, runtimeProcess.getExitValue());
 	}
+
+	/**
+	 * Test {@link RuntimeProcess} terminating the wrapped process and its
+	 * descendants.
+	 */
+	@Test
+	public void testTerminateProcessWithSubProcesses() throws Exception {
+
+		MockProcess grandChildProcess = new MockProcess(MockProcess.RUN_FOREVER);
+
+		MockProcess childProcess1 = new MockProcess(MockProcess.RUN_FOREVER);
+		childProcess1.setHandle(new MockProcessHandle(childProcess1, List.of(grandChildProcess)));
+
+		MockProcess childProcess2 = new MockProcess(MockProcess.RUN_FOREVER);
+
+		MockProcess mockProcess = new MockProcess(MockProcess.RUN_FOREVER);
+		mockProcess.setHandle(new MockProcessHandle(childProcess1, List.of(childProcess1, childProcess2)));
+
+		RuntimeProcess runtimeProcess = mockProcess.toRuntimeProcess();
+
+		assertTrue("RuntimeProcess already terminated.", grandChildProcess.isAlive());
+		assertTrue("RuntimeProcess already terminated.", childProcess1.isAlive());
+		assertTrue("RuntimeProcess already terminated.", childProcess2.isAlive());
+		assertFalse("RuntimeProcess already terminated.", runtimeProcess.isTerminated());
+
+		runtimeProcess.terminate();
+
+		assertFalse("RuntimeProcess failed to terminate wrapped process.", mockProcess.isAlive());
+		assertFalse("RuntimeProcess failed to terminate child of wrapped process.", childProcess1.isAlive());
+		assertFalse("RuntimeProcess failed to terminate child of wrapped process.", childProcess2.isAlive());
+		assertFalse("RuntimeProcess failed to terminate descendant of wrapped process.", grandChildProcess.isAlive());
+
+		TestUtil.waitWhile(p -> !p.isTerminated(), runtimeProcess, 1000, p -> "RuntimePocess not terminated.");
+	}
+
+	/**
+	 * Test {@link RuntimeProcess} terminating the wrapped process which does
+	 * not support {@link Process#toHandle()}.
+	 */
+	@Test
+	public void testTerminateProcessNotSupportingProcessToHandle() throws Exception {
+
+		MockProcess mockProcess = new MockProcess(MockProcess.RUN_FOREVER);
+		// set handle to null, so the standard java.lang.Process.toHandle()
+		// implementation is called which throws an
+		// UnsupportedOperationException
+		mockProcess.setHandle(null);
+		assertThrows(UnsupportedOperationException.class, () -> mockProcess.toHandle());
+		RuntimeProcess runtimeProcess = mockProcess.toRuntimeProcess();
+		runtimeProcess.terminate(); // must not throw, even toHandle() does
+
+		TestUtil.waitWhile(p -> !p.isTerminated(), runtimeProcess, 1000, p -> "RuntimePocess not terminated.");
+	}
 }