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

Bug 344892

Summary: [remote][multi-process] Deadlock when trying to attach to more than one process on a remote target
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, pawel.1.piech
Version: 8.0Flags: pawel.1.piech: review+
Target Milestone: 8.0   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Use a job instead of locking the executor
marc.khouzam: iplog-
Use a job instead of locking the executor (2)
marc.khouzam: iplog-
Patch using UIJob pawel.1.piech: iplog-

Description Marc Khouzam CLA 2011-05-05 15:46:24 EDT
From bug 340535 comment #1:

> For the remote attach case, I ran into a deadlock.  The reason is that for a
> remote attach, the connect action (GdbConnectCommand) needs to prompt the user
> for the path to the binary (not needed for local attach), which can deadlock
> because GdbReverseToggleCommand also locks the UI thread.

To reproduce:
1- start gdbserver --multi :9999
2- start a remote attach session
3- press 'connect' and choose a binary
4- press 'connect' again, and choose a binary (the same one is fine)
=> there is a deadlock as described above.
Comment 1 Marc Khouzam CLA 2011-05-05 16:01:53 EDT
The deadlock is because the connect command, which is running in the DSF executor needs to UI thread to prompt the user for the path of the binary; while at the same time, the ReverseDebuggingPropertyTester, calls GdbReverseToggleCommand.isReverseToggled() on the UI thread and locks it waiting for the DSF executor.
Comment 2 Marc Khouzam CLA 2011-05-05 16:25:15 EDT
Created attachment 194880 [details]
Use a job instead of locking the executor

This patch uses a job to prompt the user for the binary path.
Comment 3 Marc Khouzam CLA 2011-05-05 21:55:54 EDT
Created attachment 194895 [details]
Use a job instead of locking the executor (2)

Update on the patch to properly complete the RequestMonitor.
Comment 4 Marc Khouzam CLA 2011-05-05 22:04:56 EDT
I committed the last patch to HEAD.

Pawel can you review?  The patch simply moves the long prompting operation to a job so it can free the executor.
Comment 5 CDT Genie CLA 2011-05-05 22:23:03 EDT
*** cdt cvs genie on behalf of mkhouzam ***
Bug 344892: Deadlock when trying to attach to more than one process on a remote target

[*] GdbConnectCommand.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbConnectCommand.java?root=Tools_Project&r1=1.7&r2=1.8
Comment 6 Pawel Piech CLA 2011-05-06 19:08:31 EDT
Wow, I had to span across two screens to see the whole patch ;-)

The fix looks fine to me.  If you want it a little cleaner, you can use a UIJob, which internally does a Display.syncExec().
Comment 7 Pawel Piech CLA 2011-05-06 19:09:02 EDT
Created attachment 194995 [details]
Patch using UIJob
Comment 8 Marc Khouzam CLA 2011-05-06 20:10:28 EDT
(In reply to comment #7)
> Created attachment 194995 [details]
> Patch using UIJob

Much appreciated, I will use it!
Comment 9 Marc Khouzam CLA 2011-05-07 20:34:33 EDT
(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 194995 [details] [details]
> > Patch using UIJob
> 
> Much appreciated, I will use it!

I committed Pawel patch to HEAD.  Thanks again.
Comment 10 CDT Genie CLA 2011-05-07 21:23:04 EDT
*** cdt cvs genie on behalf of mkhouzam ***
Bug 344892: Deadlock when trying to attach to more than one process on a remote target

[*] GdbConnectCommand.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbConnectCommand.java?root=Tools_Project&r1=1.8&r2=1.9
Comment 11 CDT Genie CLA 2011-05-08 20:23:04 EDT
*** cdt cvs genie on behalf of mkhouzam ***
Bug 344892: Deadlock when trying to attach to more than one process on a remote target.  Small cleanup

[*] GdbConnectCommand.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbConnectCommand.java?root=Tools_Project&r1=1.9&r2=1.10