Console View: fix concurrency Issues

Running console did sometimes not show start infos transfered by
Launch.fAttributes between threads

eclipse-jdt/eclipse.jdt.debug#390

Author: EcljpseB0T

debug/org.eclipse.debug.core/core/org/eclipse/debug/core/Launch.java

@@ -17,8 +17,10 @@
 
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -66,7 +68,7 @@ public class Launch extends PlatformObject implements ILaunch, IDisconnect, ILau
 	/**
 	 * The configuration that was launched, or null.
 	 */
-	private ILaunchConfiguration fConfiguration= null;
+	private volatile ILaunchConfiguration fConfiguration = null;
 
 	/**
 	 * The system processes associated with
@@ -78,7 +80,7 @@ public class Launch extends PlatformObject implements ILaunch, IDisconnect, ILau
 	 * The source locator to use in the debug session
 	 * or <code>null</code> if not supported.
 	 */
-	private ISourceLocator fLocator= null;
+	private volatile ISourceLocator fLocator = null;
 
 	/**
 	 * The mode this launch was launched in.
@@ -88,7 +90,7 @@ public class Launch extends PlatformObject implements ILaunch, IDisconnect, ILau
 	/**
 	 * Table of client defined attributes
 	 */
-	private HashMap<String, String> fAttributes;
+	private final Map<String, String> fAttributes = new ConcurrentHashMap<>();
 
 	/**
 	 * Flag indicating that change notification should
@@ -319,20 +321,20 @@ public ILaunchConfiguration getLaunchConfiguration() {
 	 */
 	@Override
 	public void setAttribute(String key, String value) {
-		if (fAttributes == null) {
-			fAttributes = new HashMap<>(5);
+		Objects.requireNonNull(key);
+		if (value == null) {
+			// ConcurrentHashMap does not allow null values
+			fAttributes.remove(key);
+		} else {
+			fAttributes.put(key, value);
 		}
-		fAttributes.put(key, value);
 	}
 
 	/**
 	 * @see ILaunch#getAttribute(String)
 	 */
 	@Override
 	public String getAttribute(String key) {
-		if (fAttributes == null) {
-			return null;
-		}
 		return fAttributes.get(key);
 	}
 
@@ -343,7 +345,7 @@ public String getAttribute(String key) {
 	public IDebugTarget[] getDebugTargets() {
 		readLock.lock();
 		try {
-			return fTargets.toArray(new IDebugTarget[fTargets.size()]);
+			return fTargets.toArray(IDebugTarget[]::new);
 		} finally {
 			readLock.unlock();
 		}

debug/org.eclipse.debug.core/core/org/eclipse/debug/core/model/RuntimeProcess.java

@@ -17,9 +17,10 @@
 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.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -68,7 +69,9 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	private Process fProcess;
 
 	/**
-	 * This process's exit value
+	 * This process's exit value.
+	 *
+	 * synchronized by this
 	 */
 	private int fExitValue;
 
@@ -96,17 +99,17 @@ public class RuntimeProcess extends PlatformObject implements IProcess {
 	/**
 	 * Table of client defined attributes
 	 */
-	private Map<String, String> fAttributes;
+	private final Map<String, String> fAttributes = new ConcurrentHashMap<>();
 
 	/**
 	 * Whether output from the process should be captured or swallowed
 	 */
-	private boolean fCaptureOutput = true;
+	private final boolean fCaptureOutput;
 
 	/**
 	 * Whether the descendants of this process should be terminated too
 	 */
-	private boolean fTerminateDescendants = true;
+	private final boolean fTerminateDescendants;
 
 	private final String fThreadNameSuffix;
 
@@ -141,14 +144,16 @@ public RuntimeProcess(ILaunch launch, Process process, String name, Map<String,
 		String captureOutput = launch.getAttribute(DebugPlugin.ATTR_CAPTURE_OUTPUT);
 		fCaptureOutput = !("false".equals(captureOutput)); //$NON-NLS-1$
 
+		boolean terminateDescendants = true;
 		try {
 			ILaunchConfiguration launchConfiguration = launch.getLaunchConfiguration();
 			if (launchConfiguration != null) {
-				fTerminateDescendants = launchConfiguration.getAttribute(DebugPlugin.ATTR_TERMINATE_DESCENDANTS, true);
+				terminateDescendants = launchConfiguration.getAttribute(DebugPlugin.ATTR_TERMINATE_DESCENDANTS, true);
 			}
 		} catch (CoreException e) {
 			DebugPlugin.log(e);
 		}
+		fTerminateDescendants = terminateDescendants;
 		fThreadNameSuffix = getPidInfo(process, launch);
 
 		fStreamsProxy = createStreamsProxy();
@@ -264,7 +269,9 @@ public void terminate() throws DebugException {
 				try { // (in total don't wait longer than TERMINATION_TIMEOUT)
 					long waitStart = System.currentTimeMillis();
 					if (process.waitFor(TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) {
-						fExitValue = process.exitValue();
+						synchronized (this) {
+							fExitValue = process.exitValue();
+						}
 						if (waitFor(descendants, waitStart)) {
 							return;
 						}
@@ -419,26 +426,25 @@ protected void fireChangeEvent() {
 	 */
 	@Override
 	public void setAttribute(String key, String value) {
-		if (fAttributes == null) {
-			fAttributes = new HashMap<>(5);
-		}
-		Object origVal = fAttributes.get(key);
-		if (origVal != null && origVal.equals(value)) {
-			return; //nothing changed.
+		Objects.requireNonNull(key);
+		if (value == null) {
+			// ConcurrentHashMap does not allow null values
+			if (fAttributes.remove(key) != null) {
+				fireChangeEvent();
+			}
+		} else {
+			String origVal = fAttributes.put(key, value);
+			if (!Objects.equals(origVal, value)) {
+				fireChangeEvent();
+			}
 		}
-
-		fAttributes.put(key, value);
-		fireChangeEvent();
 	}
 
 	/**
 	 * @see IProcess#getAttribute(String)
 	 */
 	@Override
 	public String getAttribute(String key) {
-		if (fAttributes == null) {
-			return null;
-		}
 		return fAttributes.get(key);
 	}
 

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

@@ -248,8 +248,6 @@ public ProcessConsole(IProcess process, IConsoleColorProvider colorProvider, Str
 
 		colorProvider.connect(fProcess, this);
 
-		setName(computeName());
-
 		Color color = fColorProvider.getColor(IDebugUIConstants.ID_STANDARD_INPUT_STREAM);
 		if (fInput instanceof IOConsoleInputStream) {
 			((IOConsoleInputStream)fInput).setColor(color);
@@ -556,6 +554,9 @@ private synchronized void disposeStreams() {
 	protected void init() {
 		super.init();
 		DebugPlugin.getDefault().addDebugEventListener(this);
+		// computeName() after addDebugEventListener()
+		// see https://github.com/eclipse-jdt/eclipse.jdt.debug/issues/390
+		setName(computeName());
 		if (fProcess.isTerminated()) {
 			closeStreams();
 			resetName();