Bug 567345 follow-up: Remove unnecessary code in model.RuntimeProcessY20201113-1200Y20201111-0210Y20201110-1200I20201116-0600I20201115-1800I20201115-0600I20201114-1800I20201114-0740I20201114-0600I20201113-1800I20201112-1800I20201111-1800I20201110-1800I20201109-1800

Mainly simplify ProcessMonitorThread:

- Made it private (was package-private and not used in the package).

- Use the enclosing RuntimeProcess instance instead of passing 'this'.

- Do not check if fOSProcess respectively RuntimeProcess.this.fProcess
is null in a loop because at the end of the first pass in the finally
block both were/are set to null anyway (fOSProcess directly or fProcess
in RuntimeProcess.this.terminated())
- only check initially if os-process is null (null may be passed to
RuntimeProcess)

- simplify killThread() by just setting the (now volatile) fExit flag
and interrupting the thread in any case. The thread either notices
fExit=true (when killed before or immediately after becoming alive) or
the interruption (when already being alive).

All removed fields and methods were private or package private (or in a
package private class), so no API is changed.

- do not set 'fMonitor' field to null, in RuntimeProcess.terminate(). It
isn't nulled if the process terminates regularly either and their is no
semantic in doing it. And so the field can be final.

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

1 files changed, 19 insertions, 59 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 618b5cf64..bef44820a 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
@@ -20,7 +20,6 @@ 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.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -75,7 +74,7 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	 * The monitor which listens for this runtime process' system process
 	 * to terminate.
 	 */
-	private ProcessMonitorThread fMonitor;
+	private final ProcessMonitorThread fMonitor;
 
 	/**
 	 * The streams proxy for this process
@@ -116,20 +115,20 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	public RuntimeProcess(ILaunch launch, Process process, String name, Map<String, String> attributes) {
 		setLaunch(launch);
 		initializeAttributes(attributes);
-		fProcess= process;
-		fName= name;
-		fTerminated= true;
+		fProcess = process;
+		fName = name;
+		fTerminated = true;
 		try {
 			fExitValue = process.exitValue();
 		} catch (IllegalThreadStateException e) {
-			fTerminated= false;
+			fTerminated = false;
 		}
 
 		String captureOutput = launch.getAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT);
 		fCaptureOutput = !("false".equals(captureOutput)); //$NON-NLS-1$
 
-		fStreamsProxy= createStreamsProxy();
-		fMonitor = new ProcessMonitorThread(this);
+		fStreamsProxy = createStreamsProxy();
+		fMonitor = new ProcessMonitorThread();
 		fMonitor.start();
 		launch.addProcess(this);
 		fireCreationEvent();
@@ -142,9 +141,7 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	 */
 	private void initializeAttributes(Map<String, String> attributes) {
 		if (attributes != null) {
-			for (Entry<String, String> entry : attributes.entrySet()) {
-				setAttribute(entry.getKey(), entry.getValue());
-			}
+			attributes.forEach(this::setAttribute);
 		}
 	}
 
@@ -237,10 +234,7 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 			}
 
 			// clean-up
-			if (fMonitor != null) {
-				fMonitor.killThread();
-				fMonitor = null;
-			}
+			fMonitor.killThread();
 			IStatus status = new Status(IStatus.ERROR, DebugPlugin.getUniqueIdentifier(), DebugException.TARGET_REQUEST_FAILED, DebugCoreMessages.RuntimeProcess_terminate_failed, null);
 			throw new DebugException(status);
 		}
@@ -425,6 +419,7 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 		}
 		return super.getAdapter(adapter);
 	}
+
 	/**
 	 * @see IProcess#getExitValue()
 	 */
@@ -440,68 +435,38 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	 * Monitors a system process, waiting for it to terminate, and
 	 * then notifies the associated runtime process.
 	 */
-	class ProcessMonitorThread extends Thread {
+	private class ProcessMonitorThread extends Thread {
 
 		/**
 		 * Whether the thread has been told to exit.
 		 */
-		protected boolean fExit;
-		/**
-		 * The underlying <code>java.lang.Process</code> being monitored.
-		 */
-		protected Process fOSProcess;
-		/**
-		 * The <code>IProcess</code> which will be informed when this
-		 * monitor detects that the underlying process has terminated.
-		 */
-		protected RuntimeProcess fRuntimeProcess;
-
-		/**
-		 * The <code>Thread</code> which is monitoring the underlying process.
-		 */
-		protected Thread fThread;
-
-		/**
-		 * A lock protecting access to <code>fThread</code>.
-		 */
-		private final Object fThreadLock = new Object();
+		private volatile boolean fExit;
 
 		/**
 		 * @see Thread#run()
 		 */
 		@Override
 		public void run() {
-			synchronized (fThreadLock) {
-				if (fExit) {
-					return;
-				}
-				fThread = Thread.currentThread();
-			}
-			while (fOSProcess != null) {
+			Process fOSProcess = RuntimeProcess.this.getSystemProcess();
+			if (!fExit && fOSProcess != null) {
 				try {
 					fOSProcess.waitFor();
 				} catch (InterruptedException ie) {
 					// clear interrupted state
 					Thread.interrupted();
 				} finally {
-					fOSProcess = null;
-					fRuntimeProcess.terminated();
+					RuntimeProcess.this.terminated();
 				}
 			}
-			fThread = null;
 		}
 
 		/**
 		 * Creates a new process monitor and starts monitoring the process for
 		 * termination.
-		 *
-		 * @param process process to monitor for termination
 		 */
-		public ProcessMonitorThread(RuntimeProcess process) {
+		private ProcessMonitorThread() {
 			super(DebugCoreMessages.ProcessMonitorJob_0);
 			setDaemon(true);
-			fRuntimeProcess= process;
-			fOSProcess= process.getSystemProcess();
 		}
 
 		/**
@@ -511,14 +476,9 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 		 * case of an underlying process which has not informed this
 		 * monitor of its termination.
 		 */
-		protected void killThread() {
-			synchronized (fThreadLock) {
-				if (fThread == null) {
-					fExit = true;
-				} else {
-					fThread.interrupt();
-				}
-			}
+		private void killThread() {
+			fExit = true;
+			this.interrupt(); // ignored if monitor thread is not yet running
 		}
 	}
 }