Bug 347514: Tracepoint visualization should be prepared for multi-threaded programs

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<ITraceRecordDMContext>(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<String, String>());
-       							            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<String, String>());
+						            
+						            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<String, String>());
+						            		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<IContainerDMContext> fProcRestarting = new HashSet<IContainerDMContext>();
 
+	/** 
+	 * 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<IDMContext[]> 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<MIInfo>(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 {