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

7 files changed, 126 insertions, 23 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/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<IContainerDMContext> fProcRestarting = new HashSet<IContainerDMContext>();
 
+	/*
+	 * 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<IDMContext[]> 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<MIThreadInfoInfo>(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 {