From fa56c9494a899c5b48252831e748be3cbd300c6b Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sun, 15 May 2011 02:47:01 +0000 Subject: Bug 345749: MIInferior process gets terminated right away on a restart --- .../eclipse/cdt/dsf/gdb/service/GDBProcesses.java | 8 ++- .../cdt/dsf/gdb/service/GDBProcesses_7_0.java | 27 +++++++++- .../cdt/dsf/gdb/service/GDBProcesses_7_2.java | 22 ++++++-- .../dsf/mi/service/command/MIInferiorProcess.java | 62 +++++++++++++++++----- 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 attributes, final DataRequestMonitor rm) { + public void restart(IContainerDMContext containerDmc, final Map attributes, final DataRequestMonitor 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(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(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 attributes, DataRequestMonitor 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(); + } } } } -- cgit v1.2.3