Bug 464056 - Wrong exit code returned in some multi-process cases

Change-Id: I71c3aca8c8c98494408d2a7677d2837768592b09
Signed-off-by: Marc Khouzam <marc.khouzam@ericsson.com>

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_3.java

@@ -0,0 +1,39 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Ericsson and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ * 
+ * Contributors:
+ *     Marc Khouzam (Ericsson) - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.dsf.gdb.service;
+
+import java.util.Map;
+
+import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
+import org.eclipse.cdt.dsf.concurrent.DsfExecutor;
+import org.eclipse.cdt.dsf.concurrent.Sequence;
+import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext;
+import org.eclipse.cdt.dsf.service.DsfSession;
+
+/**
+ * Version for GDB 7.3
+ * 
+ * @since 4.7
+ */
+public class GDBProcesses_7_3 extends GDBProcesses_7_2_1 {
+    
+	public GDBProcesses_7_3(DsfSession session) {
+		super(session);
+	}
+
+	@Override
+	protected Sequence getStartOrRestartProcessSequence(DsfExecutor executor, IContainerDMContext containerDmc, 
+			Map<String, Object> attributes, boolean restart, 
+			DataRequestMonitor<IContainerDMContext> rm) {
+		return new StartOrRestartProcessSequence_7_3(executor, containerDmc, attributes, restart, rm);
+	}
+}
+

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_4.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014 Ericsson and others.
+ * Copyright (c) 2014, 2015 Ericsson and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -19,7 +19,7 @@ import org.eclipse.cdt.dsf.service.DsfSession;
  * 
  * @since 4.4
  */
-public class GDBProcesses_7_4 extends GDBProcesses_7_2_1 {
+public class GDBProcesses_7_4 extends GDBProcesses_7_3 {
     
 	public GDBProcesses_7_4(DsfSession session) {
 		super(session);

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GdbDebugServicesFactory.java

@@ -224,6 +224,9 @@ public class GdbDebugServicesFactory extends AbstractDsfDebugServicesFactory {
 		if (GDB_7_4_VERSION.compareTo(fVersion) <= 0) {
 			return new GDBProcesses_7_4(session);
 		}
+		if (GDB_7_3_VERSION.compareTo(fVersion) <= 0) {
+			return new GDBProcesses_7_3(session);
+		}
 		if (GDB_7_2_1_VERSION.compareTo(fVersion) <= 0) {
 			return new GDBProcesses_7_2_1(session);
 		}

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_0.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2012 Ericsson and others.
+ * Copyright (c) 2011, 2015 Ericsson and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -13,6 +13,7 @@
 package org.eclipse.cdt.dsf.gdb.service;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -291,6 +292,16 @@ public class StartOrRestartProcessSequence_7_0 extends ReflectionSequence {
     	}
     }
 	
+	/** @since 4.7 */
+	protected MIInferiorProcess createInferiorProcess(IContainerDMContext container, OutputStream outputStream) {
+		return new MIInferiorProcess(container, outputStream);
+	}
+
+	/** @since 4.7 */
+	protected MIInferiorProcess createInferiorProcess(IContainerDMContext container, PTY pty) {
+		return new MIInferiorProcess(container, pty);
+	}
+
 	/**
 	 * Before running the program, we must create its console for IO.
 	 */
@@ -305,9 +316,9 @@ public class StartOrRestartProcessSequence_7_0 extends ReflectionSequence {
     	
 		Process inferiorProcess;
 		if (fPty == null) {
-			inferiorProcess = new MIInferiorProcess(fContainerDmc, fBackend.getMIOutputStream());
+			inferiorProcess = createInferiorProcess(fContainerDmc, fBackend.getMIOutputStream());
 		} else {
-			inferiorProcess = new MIInferiorProcess(fContainerDmc, fPty);
+			inferiorProcess = createInferiorProcess(fContainerDmc, fPty);
 		}
 
 		final Process inferior = inferiorProcess;

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_3.java

@@ -0,0 +1,43 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Ericsson and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ * 
+ * Contributors:
+ *     Marc Khouzam (Ericsson) - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.dsf.gdb.service;
+
+import java.io.OutputStream;
+import java.util.Map;
+
+import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
+import org.eclipse.cdt.dsf.concurrent.DsfExecutor;
+import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext;
+import org.eclipse.cdt.dsf.mi.service.command.MIInferiorProcess;
+import org.eclipse.cdt.dsf.mi.service.command.MIInferiorProcess_7_3;
+import org.eclipse.cdt.utils.pty.PTY;
+
+/**
+ * Specialization for GDB >= 7.3 
+ * @since 4.7
+ */
+public class StartOrRestartProcessSequence_7_3 extends StartOrRestartProcessSequence_7_0 {
+
+	public StartOrRestartProcessSequence_7_3(DsfExecutor executor, IContainerDMContext containerDmc,
+			Map<String, Object> attributes, boolean restart, DataRequestMonitor<IContainerDMContext> rm) {
+		super(executor, containerDmc, attributes, restart, rm);
+	}
+
+	@Override
+	protected MIInferiorProcess createInferiorProcess(IContainerDMContext container, OutputStream outputStream) {
+		return new MIInferiorProcess_7_3(container, outputStream);
+	}
+
+	@Override
+	protected MIInferiorProcess createInferiorProcess(IContainerDMContext container, PTY pty) {
+		return new MIInferiorProcess_7_3(container, pty);
+	}
+}

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2013 QNX Software Systems and others.
+ * Copyright (c) 2009, 2015 QNX Software Systems and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -42,12 +42,9 @@ import org.eclipse.cdt.dsf.debug.service.command.ICommandListener;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandResult;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandToken;
 import org.eclipse.cdt.dsf.debug.service.command.IEventListener;
-import org.eclipse.cdt.dsf.gdb.IGdbDebugConstants;
 import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin;
-import org.eclipse.cdt.dsf.gdb.launching.InferiorRuntimeProcess;
 import org.eclipse.cdt.dsf.mi.service.IMICommandControl;
 import org.eclipse.cdt.dsf.mi.service.IMIContainerDMContext;
-import org.eclipse.cdt.dsf.mi.service.MIProcesses;
 import org.eclipse.cdt.dsf.mi.service.command.commands.CLICommand;
 import org.eclipse.cdt.dsf.mi.service.command.events.MIThreadGroupExitedEvent;
 import org.eclipse.cdt.dsf.mi.service.command.output.MIGDBShowExitCodeInfo;
@@ -61,13 +58,11 @@ import org.eclipse.cdt.utils.pty.PTY;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
-import org.eclipse.debug.core.ILaunch;
-import org.eclipse.debug.core.model.IProcess;
 
 /**
- * This Process implementation tracks the process that is being debugged 
+ * This Process implementation tracks one of the inferiors that is being debugged 
- * by GDB.  The process object is displayed in Debug view and is used to
+ * by GDB.  The process object, although not displayed in the Debug view, is used to
- * channel the STDIO of the interior process to the console view.  
+ * channel the STDIO of the inferior process to the console view.
  * 
  * @see org.eclipse.debug.core.model.IProcess 
  */
@@ -119,6 +114,27 @@ public class MIInferiorProcess extends Process
     @ThreadSafe
     Integer fExitCode = null;
     
+    /**
+     * @returns if that the inferior has been started.
+	 * This is important for the case of a restart
+	 * where we need to make sure not to terminate
+	 * the new inferior, which was not started yet.
+	 * @since 4.7
+	 */
+    protected boolean isStarted() {
+		return fStarted;
+	}
+ 
+    /** @since 4.7 */
+    protected IContainerDMContext getContainer() {
+		return fContainerDMContext;
+	}
+    
+    /** @since 4.7 */
+    protected synchronized boolean isTerminated() {
+		return fTerminated;
+	}
+    
     /**
      * Creates an inferior process object which uses the given output stream 
      * to write the user standard input into.
@@ -145,8 +161,9 @@ public class MIInferiorProcess extends Process
         this(container, null, p);
     }
     
+    /** @since 4.7 */
     @ConfinedToDsfExecutor("fSession#getExecutor")
-    private MIInferiorProcess(IContainerDMContext container, final OutputStream gdbOutputStream, PTY pty) {
+    protected MIInferiorProcess(IContainerDMContext container, final OutputStream gdbOutputStream, PTY pty) {
         fSession = DsfSession.getSession(container.getSessionId());
         fSession.addServiceEventListener(this, null);
         
@@ -260,11 +277,13 @@ public class MIInferiorProcess extends Process
             }
         }
         
-        // The below should only be used for GDB < 7.3 because $_exitcode does not
+        // Fetch the exit code using $_exitcode of GDB when doing single
-        // support multi-process properly.  Note that for GDB 7.2, there is no proper
+        // process debugging (GDB <= 7.1)
-        // solution because although we support multi-process, $_exitcode was still the
+        // Note that for GDB 7.2, there is no proper solution for the exit code
-        // only way to get the exit code.  For GDB >= 7.3, we can use =thread-group-exited
+        // because although we support multi-process, $_exitcode was still the
-        // which provides the exit code; in fact, fExitCode will already be set for that case.
+        // only way to get the exit code, and that variable does not work properly
+        // with multi-process (it is re-used by the different processes).
+        // We use it still for GDB 7.2, since the single-process case is the most common.
         try {
             Query<Integer> exitCodeQuery = new Query<Integer>() {
                 @Override
@@ -282,7 +301,7 @@ public class MIInferiorProcess extends Process
                     } else if (!fTerminated) {
                         // This will cause ExecutionException to be thrown with a CoreException, 
                         // which will in turn contain the IllegalThreadStateException.
-                        rm.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, IDsfStatusConstants.INVALID_STATE, "GDB is still running.", new IllegalThreadStateException())); //$NON-NLS-1$
+                        rm.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, IDsfStatusConstants.INVALID_STATE, "Inferior is still running.", new IllegalThreadStateException())); //$NON-NLS-1$
                         rm.done();
                     } else {
                     	// The exitCode from GDB does not seem to be handled for multi-process
@@ -461,28 +480,6 @@ public class MIInferiorProcess extends Process
     /** @since 4.2 */
     @DsfServiceEventHandler
     public void eventDispatched(MIThreadGroupExitedEvent e) {
-		if (fContainerDMContext instanceof IMIContainerDMContext) {
-			if (((IMIContainerDMContext)fContainerDMContext).getGroupId().equals(e.getGroupId())) {
-    			if (fStarted) {
-    				// Only handle this event if this process was already
-    				// started.  This is to protect ourselves in the case of
-    				// a restart, where the new inferior is already created
-    				// and gets the exited event for the old inferior.
-    				String exitCode = e.getExitCode();
-    				if (exitCode != null) {
-    					setExitCodeAttribute();
-    					try {
-    						// Must use 'decode' since GDB returns an octal value
-    						Integer decodedExitCode = Integer.decode(exitCode);
-			        		synchronized(this) {
-    			            	fExitCode = decodedExitCode;
-    			            }
-    					} catch (NumberFormatException exception) {
-    					}    					
-    				}
-    			}
-    		}
-    	}
     }
     
     /**
@@ -525,34 +522,4 @@ public class MIInferiorProcess extends Process
     public void eventDispatched(ICommandControlShutdownDMEvent e) {
     	dispose();
     }
-
-	/**
-	 * Set an attribute in the inferior process of the launch to indicate
-	 * that the inferior has properly exited and its exit value can be used.
-	 */
-    @ConfinedToDsfExecutor("fSession#getExecutor")
-	private void setExitCodeAttribute() {
-		// Update the console label to contain the exit code
-		ILaunch launch = (ILaunch)fSession.getModelAdapter(ILaunch.class);
-		IProcess[] launchProcesses = launch.getProcesses();
-		for (IProcess proc : launchProcesses) {
-			if (proc instanceof InferiorRuntimeProcess) {
-				InferiorRuntimeProcess process = (InferiorRuntimeProcess)proc;
-				String groupAttribute = process.getAttribute(IGdbDebugConstants.INFERIOR_GROUPID_ATTR);
-
-				// if the groupAttribute is not set in the process we know we are dealing
-				// with single process debugging so the one process is the one we want.
-				// If the groupAttribute is set, then we must make sure it is the proper inferior
-				if (fContainerDMContext instanceof IMIContainerDMContext) {
-					if (groupAttribute == null || groupAttribute.equals(MIProcesses.UNIQUE_GROUP_ID) ||
-							groupAttribute.equals(((IMIContainerDMContext)fContainerDMContext).getGroupId())) {
-						// Simply set the attribute that indicates the inferior has properly exited and its
-						// exit code can be used.
-						process.setAttribute(IGdbDebugConstants.INFERIOR_EXITED_ATTR, ""); //$NON-NLS-1$
-						return;
-					}
-				}
-			}
-		}
-	}
 }

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess_7_3.java

@@ -0,0 +1,126 @@
+/*******************************************************************************
+ * Copyright (c) 2015 Ericsson and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Marc Khouzam (Ericsson) - Initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.cdt.dsf.mi.service.command;
+
+import java.io.OutputStream;
+
+import org.eclipse.cdt.dsf.concurrent.ConfinedToDsfExecutor;
+import org.eclipse.cdt.dsf.concurrent.ThreadSafeAndProhibitedFromDsfExecutor;
+import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext;
+import org.eclipse.cdt.dsf.gdb.IGdbDebugConstants;
+import org.eclipse.cdt.dsf.gdb.launching.InferiorRuntimeProcess;
+import org.eclipse.cdt.dsf.mi.service.IMIContainerDMContext;
+import org.eclipse.cdt.dsf.mi.service.command.events.MIThreadGroupExitedEvent;
+import org.eclipse.cdt.dsf.service.DsfServiceEventHandler;
+import org.eclipse.cdt.dsf.service.DsfSession;
+import org.eclipse.cdt.utils.pty.PTY;
+import org.eclipse.debug.core.ILaunch;
+import org.eclipse.debug.core.model.IProcess;
+
+/**
+ * Specialization for GDB >= 7.3.
+ * 
+ * @since 4.7
+ */
+public class MIInferiorProcess_7_3 extends MIInferiorProcess
+{
+
+	private DsfSession fSession;
+	
+    @ConfinedToDsfExecutor("fSession#getExecutor")
+    public MIInferiorProcess_7_3(IContainerDMContext container, OutputStream gdbOutputStream) {
+        this(container, gdbOutputStream, null);
+    }
+    
+    @ConfinedToDsfExecutor("fSession#getExecutor")
+    public MIInferiorProcess_7_3(IContainerDMContext container, PTY p) {
+        this(container, null, p);
+    }
+    
+    @ConfinedToDsfExecutor("fSession#getExecutor")
+    protected MIInferiorProcess_7_3(IContainerDMContext container, final OutputStream gdbOutputStream, PTY pty) {
+        super(container, gdbOutputStream, pty);
+        fSession = DsfSession.getSession(container.getSessionId());
+    }
+
+    @ThreadSafeAndProhibitedFromDsfExecutor("fSession#getExecutor")
+    @Override
+    public int exitValue() {
+    	assert !fSession.getExecutor().isInExecutorThread();
+
+    	synchronized(this) {
+    		if (fExitCode != null) {
+    			return fExitCode;
+    		}
+    	}
+
+    	if (!isTerminated()) {
+    		// Throw an exception because the process is still running.
+   			throw new IllegalThreadStateException();
+    	}
+    	
+    	return 0;
+    }
+
+    /** @since 4.2 */
+    @Override
+	@DsfServiceEventHandler
+    public void eventDispatched(MIThreadGroupExitedEvent e) {
+		if (getContainer() instanceof IMIContainerDMContext) {
+			if (((IMIContainerDMContext)getContainer()).getGroupId().equals(e.getGroupId())) {
+    			if (isStarted()) {
+    				// Only handle this event if this process was already
+    				// started.  This is to protect ourselves in the case of
+    				// a restart, where the new inferior is already created
+    				// and gets the exited event for the old inferior.
+    				String exitCode = e.getExitCode();
+    				if (exitCode != null) {
+    					setExitCodeAttribute();
+    					try {
+    						// Must use 'decode' since GDB returns an octal value
+    						Integer decodedExitCode = Integer.decode(exitCode);
+			        		synchronized(this) {
+    			            	fExitCode = decodedExitCode;
+    			            }
+    					} catch (NumberFormatException exception) {
+    					}    					
+    				}
+    			}
+    		}
+    	}
+    }
+    
+	/**
+	 * Set an attribute in the inferior process of the launch to indicate
+	 * that the inferior has properly exited and its exit value can be used.
+	 */
+    @ConfinedToDsfExecutor("fSession#getExecutor")
+	private void setExitCodeAttribute() {
+		// Update the console label to contain the exit code
+		ILaunch launch = (ILaunch)fSession.getModelAdapter(ILaunch.class);
+		IProcess[] launchProcesses = launch.getProcesses();
+		for (IProcess proc : launchProcesses) {
+			if (proc instanceof InferiorRuntimeProcess) {
+				String groupAttribute = proc.getAttribute(IGdbDebugConstants.INFERIOR_GROUPID_ATTR);
+
+				if (getContainer() instanceof IMIContainerDMContext) {
+					if (groupAttribute != null && groupAttribute.equals(((IMIContainerDMContext)getContainer()).getGroupId())) {
+						// Simply set the attribute that indicates the inferior has properly exited and its
+						// exit code can be used.
+						proc.setAttribute(IGdbDebugConstants.INFERIOR_EXITED_ATTR, ""); //$NON-NLS-1$
+						return;
+					}
+				}
+			}
+		}
+	}
+}