From 9223452bf28e2ec766da65d4530e6e0921c90548 Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Tue, 29 Sep 2020 11:06:34 +0200 Subject: Bug 567345 follow-up: Remove unnecessary code in model.RuntimeProcess 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 --- .../eclipse/debug/core/model/RuntimeProcess.java | 78 ++++++---------------- 1 file 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 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 attributes) { if (attributes != null) { - for (Entry 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 java.lang.Process being monitored. - */ - protected Process fOSProcess; - /** - * The IProcess which will be informed when this - * monitor detects that the underlying process has terminated. - */ - protected RuntimeProcess fRuntimeProcess; - - /** - * The Thread which is monitoring the underlying process. - */ - protected Thread fThread; - - /** - * A lock protecting access to fThread. - */ - 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 } } } -- cgit v1.2.3