From bc75200199be9efcad9f7228cd65f0005d0078ac Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Tue, 2 Aug 2011 14:50:40 -0400 Subject: [PATCH] Bug 347514: Tracepoint visualization should be prepared for multi-threaded programs --- .../GdbSelectNextTraceRecordCommand.java | 24 ++++++--- .../ui/viewmodel/launch/LaunchVMProvider.java | 29 +++++++++++ .../cdt/dsf/gdb/service/GDBProcesses_7_2.java | 50 +++++++++++++++++++ .../dsf/gdb/service/GDBRunControl_7_0_NS.java | 21 +++++--- .../dsf/gdb/service/GDBRunControl_7_2_NS.java | 47 ++++++++++++++++- .../dsf/gdb/service/GDBTraceControl_7_2.java | 2 +- .../gdb/service/command/GDBControl_7_0.java | 12 ----- 7 files changed, 154 insertions(+), 31 deletions(-) diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/commands/GdbSelectNextTraceRecordCommand.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/commands/GdbSelectNextTraceRecordCommand.java index b2a8f7ef1da..16a99f54cb3 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/commands/GdbSelectNextTraceRecordCommand.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/commands/GdbSelectNextTraceRecordCommand.java @@ -80,14 +80,22 @@ public class GdbSelectNextTraceRecordCommand extends AbstractDebugCommand implem new DataRequestMonitor(fExecutor, rm) { @Override protected void handleSuccess() { - final ITraceRecordDMContext nextDmc = traceControl.createNextRecordContext(getData()); - traceControl.selectTraceRecord(nextDmc, new RequestMonitor(ImmediateExecutor.getInstance(), rm) { - @Override - protected void handleSuccess() { - fSession.dispatchEvent(new TraceRecordSelectedChangedEvent(nextDmc), new Hashtable()); - rm.done(); - } - }); + final ITraceRecordDMContext previousDmc = getData(); + ITraceRecordDMContext nextDmc = traceControl.createNextRecordContext(previousDmc); + // Must send the event right away to tell the services we are starting visualization + // If we don't, the services won't behave accordingly soon enough + // Bug 347514 + fSession.dispatchEvent(new TraceRecordSelectedChangedEvent(nextDmc), new Hashtable()); + + traceControl.selectTraceRecord(nextDmc, new RequestMonitor(ImmediateExecutor.getInstance(), rm) { + @Override + protected void handleError() { + // If we weren't able to select the next record, we must notify that we are still on the previous one + // since we have already sent a TraceRecordSelectedChangedEvent early, but it didn't happen. + fSession.dispatchEvent(new TraceRecordSelectedChangedEvent(previousDmc), new Hashtable()); + rm.done(); + } + }); }; }); } else { diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmodel/launch/LaunchVMProvider.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmodel/launch/LaunchVMProvider.java index 9b15f74b42e..f7f32928ce8 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmodel/launch/LaunchVMProvider.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmodel/launch/LaunchVMProvider.java @@ -26,6 +26,7 @@ import org.eclipse.cdt.dsf.debug.ui.viewmodel.launch.AbstractLaunchVMProvider; import org.eclipse.cdt.dsf.debug.ui.viewmodel.launch.LaunchRootVMNode; import org.eclipse.cdt.dsf.debug.ui.viewmodel.launch.StackFramesVMNode; import org.eclipse.cdt.dsf.gdb.internal.ui.GdbUIPlugin; +import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITraceRecordSelectedChangedDMEvent; import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITracingStartedDMEvent; import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITracingStoppedDMEvent; import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITracingSupportedChangeDMEvent; @@ -45,6 +46,12 @@ import org.eclipse.debug.internal.ui.viewers.model.provisional.IPresentationCont public class LaunchVMProvider extends AbstractLaunchVMProvider implements IDebugEventSetListener, ILaunchesListener2 { + + /** + * Indicates that we are currently visualizing trace data. + */ + private boolean fTracepointVisualizationModeEnabled; + @ThreadSafe public LaunchVMProvider(AbstractVMAdapter adapter, IPresentationContext presentationContext, DsfSession session) { @@ -91,6 +98,15 @@ public class LaunchVMProvider extends AbstractLaunchVMProvider return true; } } + + if (eventToSkip instanceof ITraceRecordSelectedChangedDMEvent) { + ITraceRecordSelectedChangedDMEvent recordChanged = (ITraceRecordSelectedChangedDMEvent)eventToSkip; + if (recordChanged.isVisualizationModeEnabled() == fTracepointVisualizationModeEnabled) { + // We only care about this event if it indicates a change of visualization state + return true; + } + } + return super.canSkipHandlingEvent(newEvent, eventToSkip); } @@ -107,6 +123,19 @@ public class LaunchVMProvider extends AbstractLaunchVMProvider return; } + if (event instanceof ITraceRecordSelectedChangedDMEvent) { + ITraceRecordSelectedChangedDMEvent recordChanged = (ITraceRecordSelectedChangedDMEvent)event; + // If trace visualization has changed we have to refresh the debug view + if (recordChanged.isVisualizationModeEnabled() != fTracepointVisualizationModeEnabled) { + fTracepointVisualizationModeEnabled = recordChanged.isVisualizationModeEnabled(); + + // Refresh the view because the set of threads has totally changed. + refresh(); + rm.done(); + return; + } + } + super.handleEvent(event, rm); } 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 5e66158a314..3dda911eef5 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 @@ -29,9 +29,11 @@ import org.eclipse.cdt.dsf.debug.service.IRunControl.IExitedDMEvent; import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService.ICommandControlDMContext; import org.eclipse.cdt.dsf.gdb.IGDBLaunchConfigurationConstants; import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin; +import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITraceRecordSelectedChangedDMEvent; import org.eclipse.cdt.dsf.gdb.service.command.IGDBControl; import org.eclipse.cdt.dsf.mi.service.IMICommandControl; import org.eclipse.cdt.dsf.mi.service.IMIContainerDMContext; +import org.eclipse.cdt.dsf.mi.service.IMIExecutionDMContext; import org.eclipse.cdt.dsf.mi.service.IMIProcessDMContext; import org.eclipse.cdt.dsf.mi.service.IMIRunControl; import org.eclipse.cdt.dsf.mi.service.IMIRunControl.MIRunMode; @@ -54,6 +56,12 @@ import org.eclipse.debug.core.ILaunch; */ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { + /** + * The id of the single thread to be used during event visualization. + * @since 4.1 + */ + protected static final String TRACE_VISUALIZATION_THREAD_ID = "1"; //$NON-NLS-1$ + private CommandFactory fCommandFactory; private IGDBControl fCommandControl; private IGDBBackend fBackend; @@ -74,6 +82,11 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { */ private Set fProcRestarting = new HashSet(); + /** + * Indicates that we are currently visualizing trace data. + */ + private boolean fTraceVisualization; + public GDBProcesses_7_2(DsfSession session) { super(session); } @@ -109,6 +122,16 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { super.shutdown(requestMonitor); } + /** @since 4.1 */ + protected boolean getTraceVisualization() { + return fTraceVisualization; + } + + /** @since 4.1 */ + protected void setTraceVisualization(boolean visualizing) { + fTraceVisualization = visualizing; + } + @Override public IMIContainerDMContext createContainerContextFromGroupId(ICommandControlDMContext controlDmc, String groupId) { String pid = getGroupToPidMap().get(groupId); @@ -459,6 +482,25 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { return new DebugNewProcessSequence_7_2(executor, isInitial, dmc, file, attributes, rm); } + @Override + public void getProcessesBeingDebugged(final IDMContext dmc, final DataRequestMonitor rm) { + if (getTraceVisualization()) { + // If we are visualizing data during a live session, we should not ask GDB for the list of threads, + // because we will get the list of active threads, while GDB only cares about thread 1 for visualization. + final IMIContainerDMContext containerDmc = DMContexts.getAncestorOfType(dmc, IMIContainerDMContext.class); + if (containerDmc != null) { + IProcessDMContext procDmc = DMContexts.getAncestorOfType(containerDmc, IProcessDMContext.class); + rm.setData(new IMIExecutionDMContext[]{createExecutionContext(containerDmc, + createThreadContext(procDmc, TRACE_VISUALIZATION_THREAD_ID), + TRACE_VISUALIZATION_THREAD_ID)}); + rm.done(); + return; + } + } + + super.getProcessesBeingDebugged(dmc, rm); + } + /** * Creates the container context that is to be used for the new process that will * be created by the restart operation. @@ -535,5 +577,13 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { protected boolean needFixForGDB72Bug352998() { return true; } + + /** + * @since 4.1 + */ + @DsfServiceEventHandler + public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { + setTraceVisualization(e.isVisualizationModeEnabled()); + } } diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_0_NS.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_0_NS.java index 95874c88fe8..51e3a496cca 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_0_NS.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_0_NS.java @@ -398,6 +398,16 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo super.shutdown(rm); } + /** @since 4.1 */ + protected boolean getRunControlOperationsEnabled() { + return fRunControlOperationsEnabled; + } + + /** @since 4.1 */ + protected void setRunControlOperationsEnabled(boolean runControlEnabled) { + fRunControlOperationsEnabled = runControlEnabled; + } + /////////////////////////////////////////////////////////////////////////// // AbstractDsfService /////////////////////////////////////////////////////////////////////////// @@ -1585,18 +1595,13 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo } /** + * @deprecated Tracing is only supported with GDB 7.2, so this logic + * was moved to the GDBRunControl_7_2_NS class. * @since 3.0 */ + @Deprecated @DsfServiceEventHandler public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { - if (e.isVisualizationModeEnabled()) { - // We have started looking at trace records. We can no longer - // do run control operations. - fRunControlOperationsEnabled = false; - } else { - // We stopped looking at trace data and gone back to debugger mode - fRunControlOperationsEnabled = true; - } } public void flushCache(IDMContext context) { diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_2_NS.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_2_NS.java index 95185aa36ca..43a6fb51e31 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_2_NS.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBRunControl_7_2_NS.java @@ -21,12 +21,14 @@ import org.eclipse.cdt.dsf.debug.service.IRunControl; import org.eclipse.cdt.dsf.debug.service.IRunControl2; import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService; import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin; +import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITraceRecordSelectedChangedDMEvent; import org.eclipse.cdt.dsf.mi.service.IMICommandControl; import org.eclipse.cdt.dsf.mi.service.IMIContainerDMContext; import org.eclipse.cdt.dsf.mi.service.IMIExecutionDMContext; import org.eclipse.cdt.dsf.mi.service.IMIRunControl; import org.eclipse.cdt.dsf.mi.service.command.CommandFactory; import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo; +import org.eclipse.cdt.dsf.service.DsfServiceEventHandler; import org.eclipse.cdt.dsf.service.DsfSession; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; @@ -41,7 +43,12 @@ public class GDBRunControl_7_2_NS extends GDBRunControl_7_0_NS private ICommandControlService fConnection; private CommandFactory fCommandFactory; - + + /** + * Keeps track if we are currently visualizing trace data or not + */ + private boolean fTraceVisualization; + /////////////////////////////////////////////////////////////////////////// // Initialization and shutdown /////////////////////////////////////////////////////////////////////////// @@ -81,6 +88,16 @@ public class GDBRunControl_7_2_NS extends GDBRunControl_7_0_NS super.shutdown(rm); } + /** @since 4.1 */ + protected boolean getTraceVisualization() { + return fTraceVisualization; + } + + /** @since 4.1 */ + protected void setTraceVisualization(boolean visualizing) { + fTraceVisualization = visualizing; + } + // Now that the flag --thread-group is globally supported // by GDB 7.2, we have to make sure not to use it twice. // Bug 340262 @@ -168,5 +185,31 @@ public class GDBRunControl_7_2_NS extends GDBRunControl_7_0_NS private void doResumeContainer(IMIContainerDMContext context, final RequestMonitor rm) { fConnection.queueCommand(fCommandFactory.createMIExecContinue(context), new DataRequestMonitor(getExecutor(), rm)); - } + } + + /** + * @since 4.1 + */ + @Override + @DsfServiceEventHandler + public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { + setTraceVisualization(e.isVisualizationModeEnabled()); + + // Disable or re-enable run control operations if we are looking + // at trace data or we are not, respectively. + setRunControlOperationsEnabled(!e.isVisualizationModeEnabled()); + } + + @Override + protected void refreshThreadStates() { + // We should not refresh the thread state while we are visualizing trace data. + // This is because GDB will report the state of the threads of the executing + // program, while we should only deal with a 'fake' stopped thread 1, during + // visualization. + // So, simply keep the state of the threads as is until visualization stops. + // Bug 347514 + if (getTraceVisualization() == false) { + super.refreshThreadStates(); + } + } } diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBTraceControl_7_2.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBTraceControl_7_2.java index b90b214ec84..95981034d11 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBTraceControl_7_2.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBTraceControl_7_2.java @@ -972,7 +972,7 @@ public class GDBTraceControl_7_2 extends AbstractDsfService implements IGDBTrace // process? IContainerDMContext processContainerDmc = (IContainerDMContext)(getData()[0]); - // Now find the proper thread. We must do this hear because in post-mortem debugging + // Now find the proper thread. We must do this here because in post-mortem debugging // we cannot rely on MIRunControl using 'thread', as it will fail IMIProcesses procService = getServicesTracker().getService(IMIProcesses.class); if (procService == null) { diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java index f098ca7d0b2..925bd514420 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_0.java @@ -345,18 +345,6 @@ public class GDBControl_7_0 extends AbstractMIControl implements IGDBControl { /** @since 3.0 */ @DsfServiceEventHandler public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { - if (e.isVisualizationModeEnabled()) { - // Once we start looking at trace frames, we should not use - // the --thread or --frame options because GDB does not handle - // it well, there are no actual threads running. - // We only need to do this once, but it won't hurt to do it - // every time. - setUseThreadAndFrameOptions(false); - } else { - // We stopped looking at trace frames, so we can start - // using --thread and --frame again - setUseThreadAndFrameOptions(true); - } } public static class InitializationShutdownStep extends Sequence.Step {