If a real time signal is raised 2 times whereas the signal is blocked, unblock the signal will call the signal handler twice. The C signal handler of the Python signal module only stores a boolean to say if the Python signal handler should be called or not in Py_CheckSignals().

If the C signal handler is called twice, the Python signal handler is only called once.

Attached patch is a draft to fix this issue. The patch is not completly safe.

example.py: example to demonstrate the problem. The Python signal handler is only called once, it should be called twice.

Dunno.

> The patch is not completely safe.

Yeah it will not work without atomic ops.
Unfortunately the C standart seems to go into a direction
noone understands - as if a atomic_compare_and_swap() would
not suffice!  Do you know any machine language which reflects
what that standart draft describes?  I don't.

The NSIG detection of Modules/signalmodule.c uses 64 as a fallback.
32 seems to be more reasonable.
And you test against it instead of RTMAX in the patch.
(signalmodule.c also exports Python constants RTMIN and RTMAX
even though the standart explicitely allows these values to
be non-constants;
 http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html; last time i've done anything on signals - in 2005 - that was
used nowhere - Linux, FreeBSD - though.)

Often there is a huge whole in between NSIG and RTMIN, but
struct Handlers is 8 or 12 bytes (unless the compiler does the
alignment - ouuh), so 32 unused members in Handlers[] will not
cost the world anyway; on Mac OS X (no RTSIG support?!? ;)
Python is at least 6 megabytes of memory anyway.

And does anyone actually know why the last time i looked after this
(on Linux, then) realtime signals had a default action EQ SIGABRT?
Armchair crouchers...

Is it a theoretical concern or does it affect real software?

> Is it a theoretical concern or does it affect real software?

It's more theoretical.

> Yeah it will not work without atomic ops.

... Or we can maybe block the signals (all signals or just one signal?) using pthread_sigmask(SIG_BLOCK) while we access the Handlers array. But pthread_sigmask() is not available on all OSes.

On my Linux box, Python 3.3 says that signal.NSIG is equal to 65 which looks correct.

> does anyone actually know why the last time i looked after this
> (on Linux, then) realtime signals had a default action EQ SIGABRT?

The manpage says "The default action for an unhandled real-time signal is to terminate the receiving process." It is an arbitrary choice. Why do you care about the default action?

> The NSIG detection of Modules/signalmodule.c uses 64 as a fallback.
> 32 seems to be more reasonable.
> And you test against it instead of RTMAX in the patch.

I don't understand: I don't use RTMAX in my patch.

> On my Linux box, Python 3.3 says that signal.NSIG is equal to 65
> which looks correct.

On FreeBSD NSIG only counts "old" signals (32, one 32 bit mask),
SIGRTMIN is 65 and SIGRTMAX is 126.
Our internal old signal.h states

        * If we do have realtime signals, #rtmin is 35 (i.e.,
        * #nsig, FreeBSD+) or something like 38 or even 40 (Linux),
        * and #rtmax is most likely 64 (Linux) or 128 (FreeBSD+).

so that this seems to be somewhat constant in time.
(#rtmin: we take some of those RT sigs for internal purposes if
possible.  This was maybe a bad and expensive design decision.)

> Why do you care about the default action?

* \brief Hooking program crashes (\psa crash.h crash.h\epsa).
* \note
* Installed hooks (normally) execute from within an internal
* signal handler!

So many syscalls for things which don't matter almost ever.
And that may even cost context-switches sometimes.

> I don't understand: I don't use RTMAX in my patch.

+    for (signum = 1; signum < NSIG; signum++) {

This will not catch the extended signal range on FreeBSD.

> If the C signal handler is called twice, the Python signal handler is only called once.

It's not the only shortage with the current implementation regarding (real-time) signals. Another one is that they're delivered out-of-order (lowest-numbered signal first), and the most important one - especially for real-time signals - is that the handlers are executed synchronously, when Py_CheckSignals() is called.
While we can't do much about the the later, there's at least a way to handle the other issues, a small variant of the self-pipe trick:
- in the signal module initialization code, create a pipe with both ends set to non-blocking
- in the signal handler, write the signal received to the pipe (just like what's done for wakeup_fd)
- in Py_CheckSignals(), read from the pipe: the signals will be read in order, as many times as they were received. Call the handler for each signal read from the pipe.

advantages:
- it's async-safe
- signals are handled in order
- signal handlers are called the correct number of times
- you don't have to walk through the whole array of signal handlers, since you know exatly what signals have been received

drawbacks:
- might be a bit slower, because of the read syscall
- consumes 2 FDs
- have to reinit the pipe on fork
- does Windows have pipe/read/write?
- maybe overkill

But I'm really not sure that fixing this is worth it...

By the way, to be pedantic, in the current code, wakeup_fd and Handlers should be volatile, and tripped should be sig_atomic_t.

> It's not the only shortage with the current implementation regarding
> (real-time) signals. Another one is that they're delivered
> out-of-order (lowest-numbered signal first),

Oh yes, it can be a problem.

> and the most important one - especially for real-time signals - is
> that the handlers are executed synchronously, when Py_CheckSignals()
> is called.

Evaluate Python code in a signal handler is really not a good idea! And
because of the GIL, I don't think that we can do better. But with the
GIL of Python 3.3, the Python signal handler will be called "immediatly"
before the next instruction, instead of having to wait something like
100 Python instructions (sys.getcheckinterval()). On this point, Python
3.3 is better than all previous versions.

> While we can't do much about the the later, there's at least a way to
> handle the other issues, a small variant of the self-pipe trick:
> (...)
> drawbacks:
> - might be a bit slower, because of the read syscall
> - consumes 2 FDs

Consume 2 FDs can be surprising. We cannot do that by default (just to
have in-order or real time signals).

> - have to reinit the pipe on fork

Oh yes, it can be a problem.

> - does Windows have pipe/read/write?

Do you really think that Windows supports real time signals? :) Windows
has pipes: see the Connection object of the multiprocessing module
(Antoine is working on this topic). But I don't remember if they can be
used as POSIX files (using file descriptors).

> - maybe overkill
>
> But I'm really not sure that fixing this is worth it...

It looks like your reimplements the whole signal machinery between the
kernel and the application... But anyway, we need something between the
C signal handler and the Python signal handler (because we cannot
execute Python code in a C signal handler).

Well, we can imagine to have an option/function to enable real time
and/or in-order signals (e.g. using pipes).

Can we call this mode "real time"? Real time means that we warranty a
maximum response time.

> By the way, to be pedantic, in the current code, wakeup_fd and
> Handlers should be volatile, and tripped should be sig_atomic_t.

Yes. Can you please write a patch for this?

> Evaluate Python code in a signal handler is really not a good idea!

I know, you're limited to async-safe functions, among other things :-)

> because of the GIL, I don't think that we can do better. But with the
> GIL of Python 3.3, the Python signal handler will be called "immediatly"
> before the next instruction, instead of having to wait something like
> 100 Python instructions (sys.getcheckinterval()). On this point, Python
> 3.3 is better than all previous versions.
>

Nice.

> Well, we can imagine to have an option/function to enable real time
> and/or in-order signals (e.g. using pipes).
>
> Can we call this mode "real time"? Real time means that we warranty a
> maximum response time.
>

Well, I think that we should either make this the default behaviour
(that's why I was asking whether it's possible on Windows), or not
implement it. Having two modes for signal handling is confusing.
But given that this behaviour has never been noticed before and that I
doubt that people needing real-time signals use Python (and
vice-versa), maybe we should just stick with the current
implementation, which is safe and simple (see Antoine's question, "Is
it a theoretical concern or does it affect real software?").
On the other hand, if it turns out to be interesting to some people,
I'd be happy to work on a patch.

>> By the way, to be pedantic, in the current code, wakeup_fd and
>> Handlers should be volatile, and tripped should be sig_atomic_t.
>
> Yes. Can you please write a patch for this?

Patch attached.

if you used the pipe approach you'd need to deal with the case of the write blocking (or failing if nonblocking) when the pipe buffer is full.  also you'd need to block signals around a fork and reinitialize the pipe in the child before reenabling signals.

i think all of this is overkill.  as said, this isn't a real problem.

> if you used the pipe approach you'd need to deal with the case of the
> write blocking (or failing if nonblocking) when the pipe buffer is full.

Well, a pipe is 64K on Linux (4K on older kernels). Given that each signal received consumes one byte, I'd be pretty surprised if we managed to fill the pipe before Py_CheckSignals() gets called.

> i think all of this is overkill.  as said, this isn't a real problem.

Definitely. Do you think this can be closed as "wont fix" ?

if someone comes up with a situation where this is a real problem, feel free to reopen it.

New changeset 945ca78c38b1 by Victor Stinner in branch '3.1':
Issue #12060: Use sig_atomic_t type and volatile keyword in the signal module.
http://hg.python.org/cpython/rev/945ca78c38b1

New changeset b74999f561ca by Victor Stinner in branch '3.2':
(Merge 3.1) Issue #12060: Use sig_atomic_t type and volatile keyword in the
http://hg.python.org/cpython/rev/b74999f561ca

New changeset 1aa48391da30 by Victor Stinner in branch 'default':
(Merge 3.2) Issue #12060: Use sig_atomic_t type and volatile keyword in the
http://hg.python.org/cpython/rev/1aa48391da30

> if someone comes up with a situation where this is a real problem,
> feel free to reopen it.

Ok I agree, no problem. But I commited at least Charles's patch ;-)