Bug 345749: MIInferior process gets terminated right away on a restart

4 files changed, 100 insertions, 19 deletions

diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java
index fec0ee877f9..c162628ad19 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses.java
@@ -446,7 +446,13 @@ public class GDBProcesses extends MIProcesses implements IGDBProcesses {
 			fBackend.interrupt();
 		}
 
-		startOrRestart(containerDmc, attributes, true, rm);
+		// For a restart, we are given the container context of the original process.  However, we want to start
+		// a new process with a new pid, so we should create a container for it, and not use the old container with the old pid.
+		// We must create the process dmc explicitly because using createContainerContextFromGroupId() may automatically insert
+		// the old pid.
+    	IProcessDMContext processDmc = createProcessContext(fGdb.getContext(), MIProcesses.UNKNOWN_PROCESS_ID);
+    	IContainerDMContext newContainerDmc = createContainerContext(processDmc, MIProcesses.UNIQUE_GROUP_ID);
+		startOrRestart(newContainerDmc, attributes, true, rm);
 	}
 	
 	/** @since 4.0 */
diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java
index 9a8f3a035ba..a27adb3b5a8 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java
@@ -1244,8 +1244,24 @@ public class GDBProcesses_7_0 extends AbstractDsfService
     	rm.done();
 	}
 	
+	/** 
+	 * Creates the container context that is to be used for the new process that will
+	 * be created by the restart operation.
+	 * This container does not have its pid yet, while the container of the process
+	 * that is being restarted does have its pid.
+	 * Also, for GDB 7.0 and 7.1, the groupId being the pid, we cannot use the old
+	 * container's groupId, but must use the default groupId until the pid is created.
+	 * 
+	 * @since 4.0
+	 */
+	protected IMIContainerDMContext createContainerContextForRestart(String groupId) {
+    	IProcessDMContext processDmc = createProcessContext(fCommandControl.getContext(), MIProcesses.UNKNOWN_PROCESS_ID);
+    	// Don't use the groupId passed in, since it is the old pid.
+		return createContainerContext(processDmc, MIProcesses.UNIQUE_GROUP_ID);
+	}
+	
 	/** @since 4.0 */
-	public void restart(final IContainerDMContext containerDmc, final Map<String, Object> attributes, final DataRequestMonitor<IContainerDMContext> rm) {
+	public void restart(IContainerDMContext containerDmc, final Map<String, Object> attributes, final DataRequestMonitor<IContainerDMContext> rm) {
 		fProcRestarting = true;
 		
 		// Before performing the restart, check if the process is properly suspended.
@@ -1259,12 +1275,19 @@ public class GDBProcesses_7_0 extends AbstractDsfService
 		// This required more changes than suspending the process, so it was not done
 		// just yet.
 		// Bug 246740
+
+		final String groupId = ((IMIContainerDMContext)containerDmc).getGroupId();
 		
 		// This request monitor actually performs the restart
 		RequestMonitor restartRm = new RequestMonitor(ImmediateExecutor.getInstance(), rm) {
 			@Override
 			protected void handleSuccess() {
-				startOrRestart(containerDmc, attributes, true, new DataRequestMonitor<IContainerDMContext>(ImmediateExecutor.getInstance(), rm) {
+				// For a restart, we are given the container context of the original process.  However, we want to start
+				// a new process with a new pid, so we should create a container for it, and not use the old container with the old pid.
+				// Pass in the groupId because starting with GDB 7.2, we must re-use the same groupId.
+				IContainerDMContext newContainerDmc = createContainerContextForRestart(groupId);
+
+				startOrRestart(newContainerDmc, attributes, true, new DataRequestMonitor<IContainerDMContext>(ImmediateExecutor.getInstance(), rm) {
 					@Override
 					protected void handleCompleted() {
 						if (!isSuccess()) {
diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_2.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_2.java
index 61f922f779f..73a55d3dc45 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_2.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_2.java
@@ -343,7 +343,23 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 {
 		return new DebugNewProcessSequence_7_2(executor, isInitial, dmc, file, attributes, rm);
 	}
 	
-	
+	/** 
+	 * Creates the container context that is to be used for the new process that will
+	 * be created by the restart operation.
+	 * This container does not have its pid yet, while the container of the process
+	 * that is being restarted does have its pid.
+	 * Starting with GDB 7.2, the groupId stays the same when restarting a process, so
+	 * we should re-use it; this is particularly important since we support multi-process
+	 * and we need the proper groupId
+	 * 
+	 * @since 4.0
+	 */
+	@Override
+	protected IMIContainerDMContext createContainerContextForRestart(String groupId) {
+    	IProcessDMContext processDmc = createProcessContext(fCommandControl.getContext(), MIProcesses.UNKNOWN_PROCESS_ID);
+		return createContainerContext(processDmc, groupId);
+	}
+
 	@Override
 	public void restart(final IContainerDMContext containerDmc, Map<String, Object> attributes,
             			DataRequestMonitor<IContainerDMContext> rm) {
@@ -360,9 +376,7 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 {
 		});
 	}
 	
-    /**
-     * @since 4.0
-      */
+    /** @since 4.0 */
     @DsfServiceEventHandler
     @Override
     public void eventDispatched(IExitedDMEvent e) {
diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java
index c3febf3a6e2..a87b21a9eb5 100644
--- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java
+++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/MIInferiorProcess.java
@@ -36,6 +36,7 @@ import org.eclipse.cdt.dsf.debug.service.IProcesses;
 import org.eclipse.cdt.dsf.debug.service.IProcesses.IProcessDMContext;
 import org.eclipse.cdt.dsf.debug.service.IRunControl.IContainerDMContext;
 import org.eclipse.cdt.dsf.debug.service.IRunControl.IExitedDMEvent;
+import org.eclipse.cdt.dsf.debug.service.IRunControl.IStartedDMEvent;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService.ICommandControlShutdownDMEvent;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandListener;
 import org.eclipse.cdt.dsf.debug.service.command.ICommandResult;
@@ -68,6 +69,13 @@ public class MIInferiorProcess extends Process
     implements IEventListener, ICommandListener 
 {
     
+	// Indicates 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.
+	private boolean fStarted;
+	
+	// Indicates that the inferior has been terminated
     private boolean fTerminated;
 
     private final OutputStream fOutputStream;
@@ -109,7 +117,7 @@ public class MIInferiorProcess extends Process
      * Creates an inferior process object which uses the given output stream 
      * to write the user standard input into.
      *  
-     * @param session The DsfSession this inferior belongs to
+     * @param container The process that this inferior represents
      * @param gdbOutputStream The output stream to use to write user IO into.
      * @since 4.0
      */
@@ -122,7 +130,7 @@ public class MIInferiorProcess extends Process
      * Creates an inferior process object which uses the given terminal 
      * to write the user standard input into.
      *  
-     * @param session The DsfSession this inferior belongs to
+     * @param container The process that this inferior represents
      * @param p The terminal to use to write user IO into.
      * @since 4.0
      */
@@ -411,16 +419,46 @@ public class MIInferiorProcess extends Process
     	if (e.getDMContext() instanceof IMIContainerDMContext) {
     		// For multi-process, make sure the exited event
     		// is actually for this inferior.
-    		// We must compare the groupId and not the full context
-    		// because the container that we hold is incomplete.
-    		String inferiorGroup = ((IMIContainerDMContext)fContainerDMContext).getGroupId();
-    		if (inferiorGroup == null || inferiorGroup.length() == 0) {
-    			// Single process case, so we know we have terminated
-    			setTerminated();
-    		} else {
-    			String terminatedGroup = ((IMIContainerDMContext)e.getDMContext()).getGroupId();
-    			if (inferiorGroup.equals(terminatedGroup)) {
-        			setTerminated();    				
+    		if (e.getDMContext().equals(fContainerDMContext)) {
+    			if (fStarted) {
+    				// Only mark this process as terminated if it 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.
+    				setTerminated();
+    			}
+    		}
+    	}
+    }
+    
+    /**
+	 * @since 4.0
+	 */
+    @DsfServiceEventHandler
+    public void eventDispatched(IStartedDMEvent e) {
+    	if (e.getDMContext() instanceof IMIContainerDMContext) {
+    		// Mark the inferior started if the event is for this inferior.
+    		// We may get other started events in the cases of a restarts
+    		if (!fStarted) {
+    			// For multi-process, make sure the started event
+    			// is actually for this inferior.
+    			// We must compare the groupId and not the full context
+    			// because the container that we currently hold is incomplete
+    			// because the pid was not determined yet.
+    			String inferiorGroup = ((IMIContainerDMContext)fContainerDMContext).getGroupId();
+
+    			if (inferiorGroup == null || inferiorGroup.length() == 0) {
+    				// Single process case, so we know we have started
+    				fStarted = true;
+    				// Store the fully-formed container
+    				fContainerDMContext = (IMIContainerDMContext)e.getDMContext();
+    			} else {
+    				String startedGroup = ((IMIContainerDMContext)e.getDMContext()).getGroupId();
+    				if (inferiorGroup.equals(startedGroup)) {
+    					fStarted = true;
+        				// Store the fully-formed container
+        				fContainerDMContext = (IMIContainerDMContext)e.getDMContext();
+    				}
     			}
     		}
     	}