RE: [Patch] Broken multi-process detach

Marc Khouzam
2012-07-16T19:08:56Z

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Monday, July 16, 2012 1:24 PM
> To: Marc Khouzam
> Cc: 'gdb-patches@sourceware.org'
> Subject: Re: [Patch] Broken multi-process detach
>
> On 07/16/2012 02:54 PM, Marc Khouzam wrote:
> > Hi,
> >
> > Detaching from an inferior in multi-process is broken.
> > The problem seems to be that in linux_nat.c:linux_nat_detach()
> > GDB first unregisters from the event loop by calling
> > linux_nat_async (NULL, 0). In multi-process, further
> > events received from running inferiors are missed; breakpoint
> > hits are not reported, interrupting a thread is not reported,
> > and so on. See below for a broken session example.
> >
> > We could remove the call to linux_nat_async (NULL, 0) but I didn't
> > know if that would have bad side-effects. Here is the patch that
> > does that, if it is felt that is the correct solution.
>
> I'm not thinking of any. Looks like I added it in the first
> async mode version of linux-nat.c (which was much rewritten
> later on). Let's remove it.
>
> Remove also the target_async call below:
>
> if (forks_exist_p ())
> {
> /* Multi-fork case. The current inferior_ptid is being detached
> from, but there are other viable forks to debug. Detach from
> the current fork, and context-switch to the first
> available. */
> linux_fork_detach (args, from_tty);
>
> if (non_stop && target_can_async_p ())
> target_async (inferior_event_handler, 0);
>
>
> as this was re-installing the target on the event loop if there were
> other checkpoints (checkpoint/info checkpoint) to debug. It is now
> unnecessary if we don't unregister from the event loop in the
> first place.
>
> >
> > The testsuite hangs on HEAD so I wasn't able to run it yet.
> > But I'll do so once it works again.
>
> It doesn't hang for me. I've tested the patch on x86_64
> Fedora 17, both
> sync and async modes. No regressions.

Thanks for the help!

I checked-in the following:

2012-07-16 Marc Khouzam <marc.khouzam@ericsson.com>
Pedro Alves <palves@redhat.com>

* linux-nat.c (linux_nat_detach): Don't unregister from the event
loop.

### Eclipse Workspace Patch 1.0
#P src
Index: gdb/linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.254
diff -u -r1.254 linux-nat.c
--- gdb/linux-nat.c 7 Jul 2012 12:13:56 -0000 1.254
+++ gdb/linux-nat.c 16 Jul 2012 19:04:03 -0000
@@ -1848,8 +1848,8 @@

pid = GET_PID (inferior_ptid);

- if (target_can_async_p ())
- linux_nat_async (NULL, 0);
+ /* Don't unregister from the event loop, as there may be other
+ inferiors running. */

/* Stop all threads before detaching. ptrace requires that the
thread is stopped to sucessfully detach. */
@@ -1892,9 +1892,6 @@
the current fork, and context-switch to the first
available. */
linux_fork_detach (args, from_tty);
-
- if (non_stop && target_can_async_p ())
- target_async (inferior_event_handler, 0);
}
else
linux_ops->to_detach (ops, args, from_tty);