Messages in this thread | | | Date | Fri, 25 Mar 2011 15:26:30 +0100 | From | Tejun Heo <> | Subject | Re: [PATCHSET] ptrace,signal: Improve ptrace and job control interaction |
| |
Hello, Oleg.
On Wed, Mar 23, 2011 at 07:38:37PM +0100, Oleg Nesterov wrote: > But of course we need more changes. In particular, there is still the > small problem with the CLD_CONTINUED notification. > > __ptrace_unlink() does signal_wake_up() if it adds SIGNAL_STOP_STOPPED. > This is correct, but it should also add TIF_SIGPENDING if > (signal->flags & SIGNAL_CLD_MASK) != 0. > > Otherwise, if the stopped tracee was PTRACE_CONT'ed and then SIGCONT > ends the group-stop, the real_parent won't be notified after detach.
Heh, that's an interesting one. I don't think it has much to do with __ptrace_unlink() tho. Isn't the proper solution using something akin to signal_wake_up() in SIGCONT generation path in prepare_signal()?
Explicit wake_up_state() without kick_process() is okay there because if the code assumes that the tasks are guaranteed to pass through signal delivery path whenever event worthy of notification happens (either SIGNAL_STOP_STOPPED or group_stop_count is set). PTRACE_CONT breaks that as the tracee could be running in userland and thus the solution is to add kick_process() as in signal_wake_up().
Am I making any sense?
> Unfortunately, this means that recalc_sigpending_tsk() has to check > SIGNAL_CLD_MASK as well. Do you see another solution?
Hmmm... I think the above subtle breakage exists for !ptrace case too. Please consider the following scenario.
* SIGSTOP is sent to a task and group stop is initiated.
* Before the task participates in group stop, SIGCONT is sent.
* Before CLD_STOPPED notification for the incomplete-stop/cont sequence can be made, recalc_sigpending() happens.
* CLD_STOPPED notification is pending but TIF_SIGPENDING isn't set and the task isn't in signal delivery path and can continue execution.
It's a pretty convoluted extremely unlikely corner case tho. Anyways, adding SIGNAL_CLD_MASK test to recalc_sigpending() should solve it.
> There is another case. SIGCONT can hit the stopped-but-running-task, > but I don't think we should try to set TIF_SIGPENDING in this case, > you are going to add the trap later.
Hmmm... As I wrote above, I think we should do it regardless of the new trap.
Thanks.
-- tejun
| |