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

Bug 347514

Summary: [tracepoints] Tracepoint visualization should be prepared for multi-threaded programs
Product: [Tools] CDT Reporter: Marc Khouzam <marc.khouzam>
Component: cdt-debug-dsf-gdbAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: cdtdoug, nobody, pawel.1.piech
Version: 8.0Flags: nobody: review+
Target Milestone: 8.0.1   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Potential fix for maintenance branch
marc.khouzam: iplog-
Fix for master which only returns a single thread during visualization
marc.khouzam: iplog-
Better fix preventing thread refresh during trace visualization
marc.khouzam: iplog-
Same fix for cdt_8_0 avoiding new APIs marc.khouzam: iplog-

Description Marc Khouzam CLA 2011-05-27 16:21:15 EDT
I got the following assert when trying to visualize a trace record in an application that had more than one thread.

The -list-thread-groups i1 showed all the threads, which is not what the code expects.  I was running GDB 7.2 at the time.

400,005 [MI]  124-trace-status
400,006 [MI]  124^done,supported="1",running="0",stop-reason="request",frames="15",frames-created="1\
5",buffer-size="5242880",buffer-free="5237465",disconnected="0",circular="0"
400,007 [MI]  (gdb) 
416,879 [MI]  125-list-thread-groups i1
416,883 [MI]  125^done,threads=[{id="5",target-id="Thread 7026",state="running",core="0"},{id="4",ta\
rget-id="Thread 7024",state="running",core="0"},{id="3",target-id="Thread 7023",state="running",core\
="1"},{id="2",target-id="Thread 7025",state="running",core="0"},{id="1",target-id="Thread 7006",stat\
e="running",core="0"}]


java.lang.AssertionError
	at org.eclipse.cdt.dsf.gdb.service.GDBTraceControl_7_2$8$1$1.handleSuccess(GDBTraceControl_7_2.java:977)
	at org.eclipse.cdt.dsf.concurrent.RequestMonitor.handleCompleted(RequestMonitor.java:353)
	at org.eclipse.cdt.dsf.concurrent.RequestMonitor$2.run(RequestMonitor.java:298)
	at org.eclipse.cdt.dsf.concurrent.DefaultDsfExecutor$TracingWrapperRunnable.run(DefaultDsfExecutor.java:371)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
	at java.util.concurrent.FutureTask.run(FutureTask.java:138)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:98)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:206)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
	at java.lang.Thread.run(Thread.java:662)
Comment 1 Marc Khouzam CLA 2011-06-09 16:02:20 EDT
From what I can see, once we try visualizing data, gdb handles everything through thread 1.  However, the commands to list threads, still show all the threads of execution.

In post-mortem, only one thread is shown, no matter how many where running at the time the tracepoints hit.

I think we should somehow force the thread id to 1 and the number of threads to 1 when we are visualizing data.
Comment 2 Marc Khouzam CLA 2011-06-13 15:12:11 EDT
A potential further complexity is that MI events continue to be received from GDB while we are visualizing.  So we can get a *stopped event if the program hits a breakpoint, even though we currently are not debugging.  We can get =thread-exited/=thread-started events also, even thought during visualization, we should only deal with a single thread.
Comment 3 Marc Khouzam CLA 2011-07-22 15:24:06 EDT
Created attachment 200220 [details]
Potential fix for maintenance branch

I did this fix trying to keep the changes to a minimum so it may be ok for the maintenance branch.

The idea is that if we are visualizing trace data, the method GDBProcesses_7_2.getProcessesBeingDebugged will not ask GDB for threads, but will automatically return only thread 1.

I had to add a new method to get an event in GDBProcesses_7_2 so I had to add an API filter.

I won't commit right away.
Comment 4 Marc Khouzam CLA 2011-07-26 15:33:05 EDT
A problem I found with this patch is that if thread 1 is running before we start visualization, if we stop visualization, CDT now believes the thread is stopped when it actually is not.  There is currently no way to refresh the state of the threads.

I'll have to check how the thread is marked as stopped in the RunControl service and how we could fix that.
Comment 5 Marc Khouzam CLA 2011-07-28 16:06:31 EDT
(In reply to comment #4)
> A problem I found with this patch is that if thread 1 is running before we
> start visualization, if we stop visualization, CDT now believes the thread is
> stopped when it actually is not.  There is currently no way to refresh the
> state of the threads.
> 
> I'll have to check how the thread is marked as stopped in the RunControl
> service and how we could fix that.

The reason the thread is marked as suspended is that whenever we do visualization, I used a SuspendedDMEvent for thread 1 to cause the views to refresh.  That event makes the RunControl service believe thread on was suspended.

I'll have to think about the best way to clean that up.
Comment 6 Marc Khouzam CLA 2011-08-02 11:01:49 EDT
Created attachment 200725 [details]
Fix for master which only returns a single thread during visualization

With this patch and the fix to bug 353423, going in and out of visualization properly refreshes the thread state.

So, the solution of this patch is simply to ignore what GDB says about thread lists during visualization and force a single thread with id 1.  When we stop visualizing, we go back to our internal list of threads after updating their state by asking GDB.

Committed to master.
Comment 7 Marc Khouzam CLA 2011-08-02 13:36:08 EDT
(In reply to comment #6)
> Created attachment 200725 [details]
> Fix for master which only returns a single thread during visualization
> 
> With this patch and the fix to bug 353423, going in and out of visualization
> properly refreshes the thread state.
> 
> So, the solution of this patch is simply to ignore what GDB says about thread
> lists during visualization and force a single thread with id 1.  When we stop
> visualizing, we go back to our internal list of threads after updating their
> state by asking GDB.
> 
> Committed to master.

As I was running some tests before committing, I noticed one little issue.  If thread 1 is running in the real execution, once we start visualization of trace data, although we send a fake stopped event, we will then refresh the thread states, and reset the thread to running, which prevents us from examining the trace data.

I'm thinking of either forcing the thread state to stopped during visualization, or to prevent the refreshing of the thread state during visualization.
Comment 8 Marc Khouzam CLA 2011-08-02 14:54:35 EDT
Created attachment 200757 [details]
Better fix preventing thread refresh during trace visualization

(In reply to comment #7)
> As I was running some tests before committing, I noticed one little issue.  If
> thread 1 is running in the real execution, once we start visualization of trace
> data, although we send a fake stopped event, we will then refresh the thread
> states, and reset the thread to running, which prevents us from examining the
> trace data.
> 
> I'm thinking of either forcing the thread state to stopped during
> visualization, or to prevent the refreshing of the thread state during
> visualization.

This patch still fakes a thread 1 and sends a stopped event for it during visualization of traces.  It also makes sure that if we are visualizing trace data, we don't refresh the thread states.  This is important because all services must believe thread 1 is available during trace visualization.

I'll commit this to master.
Comment 9 Marc Khouzam CLA 2011-08-02 15:16:26 EDT
Created attachment 200758 [details]
Same fix for cdt_8_0 avoiding new APIs

Here is the same fix for cdt_8_0.  To avoid adding a new API I created a special event listener in an internal package, just for this branch.

I'll commit this to cdt_8_0.
Comment 10 Marc Khouzam CLA 2011-08-02 15:18:34 EDT
Fixed

Mikhail, since you now know tracepoints, can you review when time allows?
Comment 11 CDT Genie CLA 2011-08-02 15:23:08 EDT
*** cdt git genie on behalf of Marc Khouzam ***

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

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=bc75200199be9efcad9f7228cd65f0005d0078ac
Comment 12 CDT Genie CLA 2011-08-02 16:23:05 EDT
*** cdt git genie on behalf of Marc Khouzam ***

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

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=b9269f64eba0a0951b21f2ada0a1f6aa4a270a82
Comment 13 Nobody - feel free to take it CLA 2011-08-05 14:10:48 EDT
> From what I can see, once we try visualizing data, gdb handles everything
> through thread 1.  However, the commands to list threads, still show all the
> threads of execution.
> 
Marc, what do you mean by "gdb handles everything through thread 1"?
Comment 14 Marc Khouzam CLA 2011-08-05 14:24:42 EDT
(In reply to comment #13)
> > From what I can see, once we try visualizing data, gdb handles everything
> > through thread 1.  However, the commands to list threads, still show all the
> > threads of execution.
> > 
> Marc, what do you mean by "gdb handles everything through thread 1"?

When visualizing trace data, Gdb responds to memory, variable, register commands using the selected trace record instead of the actual data from the running program.  All that data is presented as belonging to thread 1, no matter which thread it was actually collected from.  On the other even during visualization GDB thread commands still show the executing program's info.

For example, during visualization, doing 'info thread' will show all the threads of the running program, but any query to variables, etc, will fail if we use any other thread than thread 1.

Personally, I find that to be a limitation of GDB, but I was able to work around it.

Let me know if that clears things up enough or if you want more info.
Comment 15 Nobody - feel free to take it CLA 2011-08-05 18:30:41 EDT
(In reply to comment #14)
> (In reply to comment #13)
> > > From what I can see, once we try visualizing data, gdb handles everything
> > > through thread 1.  However, the commands to list threads, still show all the
> > > threads of execution.
> > > 
> > Marc, what do you mean by "gdb handles everything through thread 1"?
> 
> When visualizing trace data, Gdb responds to memory, variable, register
> commands using the selected trace record instead of the actual data from the
> running program.  All that data is presented as belonging to thread 1, no
> matter which thread it was actually collected from.  On the other even during
> visualization GDB thread commands still show the executing program's info.
> 
> For example, during visualization, doing 'info thread' will show all the
> threads of the running program, but any query to variables, etc, will fail if
> we use any other thread than thread 1.
> 
> Personally, I find that to be a limitation of GDB, but I was able to work
> around it.
> 
> Let me know if that clears things up enough or if you want more info.

Yes, that's definitely a limitation in GDB. I was just wondering if it would be useful displaying the threads in the trace visualization.
The patch seems to be OK.
Comment 16 Marc Khouzam CLA 2011-08-22 11:11:24 EDT
(In reply to comment #15)
> Yes, that's definitely a limitation in GDB. I was just wondering if it would be
> useful displaying the threads in the trace visualization.

I agree it would be nice for the user to see on what thread the data was originally collected, i.e., which thread hit the tracepoint.  I don't believe GDB 7.2 has a way to collect this information.  However, I know there was a requested enhancement to have a trace state variable for the thread id, that the user would be able to collect.  If that variable is collected, we could be smart about it and show it in the debug view.

I've opened bug 355403 about this enhancement.

> The patch seems to be OK.

Thanks for the review!