Re: [PATCH 2/3] ptrace_stop: remove the wrong ->group_stop_count bookkeeping

[Petr Tesarik <ptesarik@suse.cz>]

Oleg Nesterov wrote:
> On 01/10, Petr Tesarik wrote:
>> I can actually see a bug which may be related:
>>
>>   1. a process creates a thread (or more threads)
>>   2. I attach/detach to that thread with strace several times
>>      (each time pressing CTRL-C to quit strace)
>>   3. the whole thread group (except the traced thread) ends in
>>      TASK_STOPPED
>>
>> I looked at what strace was doing to that thread, and it sometimes sends
>> SIGSTOP shortly before detaching. This is done when the thread is
>> running, i.e. not waiting in ptrace_stop. Then PTRACE_DETACH returns
>> - -ESRCH because it requires the tracee to be stopped -- just like all
>> PTRACE_* requests except TRACEME and ATTACH. So, strace has no other
>> option than to send an explicit SIGSTOP to the thread to stop it and
>> discard it afterwards.
>>
>> Could this be related?
> 
> Perhaps yes. But there are so many oddities in this area. I don't know what
> really happens with your test-case, but afaics this can happen even without
> ptrace_stop() playing with the group stop.
> 
> Let's suppose that strace detached all sub-threads except T which is running,
> and now strace does ptrace(PTRACE_DETACH, T). This fails, so strace does
> kill(T, SIGSTOP).
> 
> Note that it use kill(), not tkill(). This means another sub-thread can
> dequeue this signal and initiate the group stop (remember, it was already
> detached and thus it is not traced any longer).

In fact, it had been never traced - I attached strace to the PID of the
sub-thread, not to the thread group leader. Anyway, I haven't seen the
erroneous stop again since I changed detach() to call tkill() instead of
kill(). It's not a proof, because the failure was very seldom, so I'll
keep testing, but it makes much sense to me.

Petr

> Now strace does wait4(T, __WALL). T notices the group stop in progress,
> calls handle_group_stop(), and notifies its parent - strace.
> 
> wait4() returns success, strace does ptrace(PTRACE_DETACH, T) again. Now
> T is TASK_STOPPED, ptrace() changes the state to TASK_TRACED and finally
> does ptrace_untrace().
> 
> ptrace_untrace() sees TASK_TRACED. But it is possible that the group stop
> is not completed yet (some sub-thread didn't pass handle_group_stop()), in
> that case we are doing signal_wake_up(T, 1) so it becomes running.
> 
> 
> I still think this series makes sense even if not complete.
> 
> Oleg.
>