Bug 1114434

Created attachment 913255 [details]
Reproducing script, should receive SIGCHLD but does not

So, after a lot of debugging, it seems to me that this is actually a bug (feature?) of pthread. The way the provided works is this:

- Python interpreter is started, has just one thread (the main thread) - this thread registers the signal handler for SIGCHLD.
- Another thread is created and run, which invokes subprocess "sleep 1".
- In the meanwhile, the main thread causes pause().
- The subprocess exits, sending SIGCHLD to its originating process. Now, there are two possibilities, depending on if the example is slightly modified or not:
  - Assuming that the example remains unmodified, then by the time the subprocess exits, the thread that invoked it still exists, hence *the child thread and not the main thread* is informed of the signal. This happens *on C level* and Python only mimics what pthread actually does. I created a pure C reproducer of this behaviour with just pthread - no Python - that I'll attach.
  - If the example is modified so that the child thread exits before the subprocess is over, then the main thread is notified, since there is no other thread to notify. Again, the C example that I'll attach in a minute behaves the same.

If anything, this is a bug in pthread signal handling, not Python itself - so I'm leaning towards closing this as notabug. If you wish, you can reassign this to glibc (provider of pthread). I think Python can't (and shouldn't) do anything about this.

Created attachment 913401 [details]
Script that reproduces the same behaviour in pure C using pthread

The attached example does not behave the same, because in the unmodified C code, the signal gets handled and in the python it does not.
(And for modified example, python with same sleeps behaves the same).

In python, since only the main thread is supposed to execute signal handlers, I would expect the main thread to handle the signals as they arrive (even when they arrive on other threads), even if means waking up the main thread.

(In reply to Dima Kuznetsov from comment #5)
> The attached example does not behave the same, because in the unmodified C
> code, the signal gets handled and in the python it does not.
> (And for modified example, python with same sleeps behaves the same).

Ah, so the problem is that in the unmodified Python example, the signal handler is never run? I'll investigate that, thanks for the clarification.

> In python, since only the main thread is supposed to execute signal
> handlers, I would expect the main thread to handle the signals as they
> arrive (even when they arrive on other threads), even if means waking up the
> main thread.

Right, the documentation does say that, so regardless of not running the handler, this is definitely a bug. I'll try to debug this some more and then open upstream patch to see what upstream thinks about it.
Thanks.

(In reply to Bohuslav "Slavek" Kabrda from comment #6)
> (In reply to Dima Kuznetsov from comment #5)
> > The attached example does not behave the same, because in the unmodified C
> > code, the signal gets handled and in the python it does not.
> > (And for modified example, python with same sleeps behaves the same).
> 
> Ah, so the problem is that in the unmodified Python example, the signal
> handler is never run? I'll investigate that, thanks for the clarification.
> 
Yes, this is exactly the issue.
> > In python, since only the main thread is supposed to execute signal
> > handlers, I would expect the main thread to handle the signals as they
> > arrive (even when they arrive on other threads), even if means waking up the
> > main thread.
> 
> Right, the documentation does say that, so regardless of not running the
> handler, this is definitely a bug. I'll try to debug this some more and then
> open upstream patch to see what upstream thinks about it.
> Thanks.
I tried debugging it a bit, if I understand correctly, 'signal_handler' marks signal as received, then adds pending call to 'checksignals_witharg' but nothing is done to wake up the main thread if it is in pause().

Thanks.

(In reply to Dima Kuznetsov from comment #7)
> (In reply to Bohuslav "Slavek" Kabrda from comment #6)
> > Ah, so the problem is that in the unmodified Python example, the signal
> > handler is never run? I'll investigate that, thanks for the clarification.
> > 
> Yes, this is exactly the issue.

> I tried debugging it a bit, if I understand correctly, 'signal_handler'
> marks signal as received, then adds pending call to 'checksignals_witharg'
> but nothing is done to wake up the main thread if it is in pause().
> 
> Thanks.

Yep, more precisely (to keep this documented):
- The way it works is that Python adds signal_handler (function defined at Modules/signalmodule.c) as a handler to the specified signal and stores the passed Python function in structure Handlers (defined at the same file).
- When the signal is received, Python adds it to a list of pending calls using Py_AddPendingCall.
- Pending calls are checked in Py_MakePendingCalls, which is called in every iteration of the bytecode evaluation loop in PyEval_EvalFrameEx.
- The problem is that since pause() isn't un-paused and hangs forever, the evaluation loop never gets to another iteration, hence can't do any pending call.

This is a really tricky situation :) I'll try to come up with a solution and I'll also report this upstream to see what can be done about it.

Reported upstream as http://bugs.python.org/issue21895

Thank you, I'll watch this.

(In reply to Dima Kuznetsov from comment #10)
> Thank you, I'll watch this.

Since the upstream maintainer proposed to mark this as a documentation problem, I'll close this bug as wontfix and suggest Docs team to document this. Dima, do you agree?

(In reply to Bohuslav "Slavek" Kabrda from comment #11)
> (In reply to Dima Kuznetsov from comment #10)
> > Thank you, I'll watch this.
> 
> Since the upstream maintainer proposed to mark this as a documentation
> problem, I'll close this bug as wontfix and suggest Docs team to document
> this. Dima, do you agree?
Yes, if this is not getting fixed, it is a good idea to have this documented.

Thanks.

I'm closing this as cantfix and requesting documentation.