Attachment 200757 Details for Bug 347514 – Better fix preventing thread refresh during trace visualization

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Better fix preventing thread refresh during trace visualization

z.patch (text/plain), 16.08 KB, created by Marc Khouzam

on 2011-08-02 14:54:35 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Marc Khouzam

Created: 2011-08-02 14:54:35 EDT

Size: 16.08 KB

patch

obsolete

>From bc75200199be9efcad9f7228cd65f0005d0078ac Tue, 2 Aug 2011 14:51:01 -0400
>From: Marc Khouzam <marc.khouzam@ericsson.com>
>Date: Tue, 2 Aug 2011 14:50:40 -0400
>Subject: [PATCH] Bug 347514: Tracepoint visualization should be prepared for multi-threaded programs
>
>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 b2a8f7e..16a99f5 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 @@
>        						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 9b15f74..f7f3292 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.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 @@
> 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 @@
>         		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 @@
>     		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 5e66158..3dda911 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.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 @@
>  */
> 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;
>@@ -73,6 +81,11 @@
>      * because we know the process will be restarted.
>      */
> 	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);
>@@ -107,6 +120,16 @@
> 	@Override
> 	public void shutdown(RequestMonitor requestMonitor) {
> 		super.shutdown(requestMonitor);
>+	}
>+	
>+	/** @since 4.1 */
>+	protected boolean getTraceVisualization() {
>+		return fTraceVisualization;
>+	}
>+
>+	/** @since 4.1 */
>+	protected void setTraceVisualization(boolean visualizing) {
>+		fTraceVisualization = visualizing;
> 	}
> 	
> 	@Override
>@@ -459,6 +482,25 @@
> 		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 @@
>     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 95874c8..51e3a49 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 @@
> 		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 @@
>     }
> 
>     /**
>+     * @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 95185aa..43a6fb5 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.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 @@
> 
> 	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 @@
> 		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 @@
> 
> 	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 b90b214..9598103 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 @@
>     										// 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 f098ca7..925bd51 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 @@
>     /** @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 {

Flags:

marc.khouzam: iplog-

Actions: View | Diff

Attachments on bug 347514: 200220 | 200725 | 200757 | 200758