From b9269f64eba0a0951b21f2ada0a1f6aa4a270a82 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Tue, 2 Aug 2011 15:10:26 -0400 Subject: [PATCH] Bug 347514: Tracepoint visualization should be prepared for multi-threaded programs --- .../GdbSelectNextTraceRecordCommand.java | 24 ++++++++---- .../ui/viewmodel/launch/LaunchVMProvider.java | 29 +++++++++++++++ ...aceRecordSelectedChangedEventListener.java | 29 +++++++++++++++ .../cdt/dsf/gdb/service/GDBProcesses_7_2.java | 37 ++++++++++++++++++- .../dsf/gdb/service/GDBRunControl_7_0_NS.java | 16 +++++++- .../dsf/gdb/service/GDBTraceControl_7_2.java | 2 +- .../gdb/service/command/GDBControl_7_0.java | 12 ------ 7 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/service/command/events/TraceRecordSelectedChangedEventListener.java 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/internal/service/command/events/TraceRecordSelectedChangedEventListener.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/service/command/events/TraceRecordSelectedChangedEventListener.java new file mode 100644 index 00000000000..fe2d9251399 --- /dev/null +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/internal/service/command/events/TraceRecordSelectedChangedEventListener.java @@ -0,0 +1,29 @@ +/******************************************************************************* + * Copyright (c) 2011 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.internal.service.command.events; + +import org.eclipse.cdt.dsf.gdb.service.IGDBTraceControl.ITraceRecordSelectedChangedDMEvent; +import org.eclipse.cdt.dsf.service.DsfServiceEventHandler; + +/** + * Temporary class within an internal package to avoid adding a new API. + * The class is an event listener for the ITraceRecordSelectedChangedDMEvent. + */ +public class TraceRecordSelectedChangedEventListener { + + public boolean fTracepointVisualizationEnabled = false; + + @DsfServiceEventHandler + public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { + fTracepointVisualizationEnabled = e.isVisualizationModeEnabled(); + } +} \ No newline at end of file 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 11aff6a66ce..c3e12892691 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 @@ -30,9 +30,11 @@ import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService.ICommand import org.eclipse.cdt.dsf.gdb.IGDBLaunchConfigurationConstants; import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin; import org.eclipse.cdt.dsf.gdb.internal.commands.MITargetDisconnect; +import org.eclipse.cdt.dsf.gdb.internal.service.command.events.TraceRecordSelectedChangedEventListener; 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; @@ -55,6 +57,11 @@ 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. + */ + private static final String TRACE_VISUALIZATION_THREAD_ID = "1"; //$NON-NLS-1$ + private CommandFactory fCommandFactory; private IGDBControl fCommandControl; private IGDBBackend fBackend; @@ -75,6 +82,11 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { */ private Set fProcRestarting = new HashSet(); + /* + * Temporary to avoid API changes + */ + private TraceRecordSelectedChangedEventListener fTraceVisualizationEventListener = new TraceRecordSelectedChangedEventListener(); + public GDBProcesses_7_2(DsfSession session) { super(session); } @@ -102,11 +114,15 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { fCommandFactory = getServicesTracker().getService(IMICommandControl.class).getCommandFactory(); fBackend = getServicesTracker().getService(IGDBBackend.class); + getSession().addServiceEventListener(fTraceVisualizationEventListener, null); + requestMonitor.done(); } @Override public void shutdown(RequestMonitor requestMonitor) { + getSession().removeServiceEventListener(fTraceVisualizationEventListener); + super.shutdown(requestMonitor); } @@ -461,6 +477,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 (fTraceVisualizationEventListener.fTracepointVisualizationEnabled) { + // 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. @@ -528,7 +563,7 @@ public class GDBProcesses_7_2 extends GDBProcesses_7_1 { * * See http://sourceware.org/ml/gdb-patches/2011-03/msg00531.html * and Bug 352998 - * */ + */ private boolean needFixForGDB72Bug352998() { return true; } 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 60dda668dd3..2835886734d 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 @@ -359,6 +359,14 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo */ private boolean fRunControlOperationsEnabled = true; + /** + * Keeps track if we are currently visualizing trace data or not. + * This will only be needed when running GDB 7.2 and tracing. + * However, we put it in this class to avoid adding new APIs to the + * maintenance branch. + */ + private boolean fTraceVisualization; + /////////////////////////////////////////////////////////////////////////// // Initialization and shutdown /////////////////////////////////////////////////////////////////////////// @@ -1590,10 +1598,14 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo @DsfServiceEventHandler public void eventDispatched(ITraceRecordSelectedChangedDMEvent e) { if (e.isVisualizationModeEnabled()) { + fTraceVisualization = true; + // We have started looking at trace records. We can no longer // do run control operations. fRunControlOperationsEnabled = false; } else { + fTraceVisualization = false; + // We stopped looking at trace data and gone back to debugger mode fRunControlOperationsEnabled = true; } @@ -1604,7 +1616,8 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo } private void refreshThreadStates() { - fConnection.queueCommand( + if (fTraceVisualization == false) { + fConnection.queueCommand( fCommandFactory.createMIThreadInfo(fConnection.getContext()), new DataRequestMonitor(getExecutor(), null) { @Override @@ -1645,6 +1658,7 @@ public class GDBRunControl_7_0_NS extends AbstractDsfService implements IMIRunCo } } }); + } } private void moveToLocation(final IExecutionDMContext context, 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 {