bug#14640: SA_RESTART prevents execution of signal handlers

bug#14640: SA_RESTART prevents execution of signal handlers

[Ludovic Courtès]

When using SA_RESTART, signal handlers are never executed, as in this
example (checked on 2.0.9+):

--8<---------------cut here---------------start------------->8---
(sigaction SIGALRM
  (lambda (signum)
    (pk 'sig signum))
  SA_RESTART)
(alarm 3)
(pk 'char (read-char))
--8<---------------cut here---------------end--------------->8---

Presumably this is because the read(2) syscall is automatically
restarted, leaving no chance for the handler async to run.

Ludo’.

bug#14640: SA_RESTART prevents execution of signal handlers

[Andy Wingo]

On Mon 17 Jun 2013 15:54, ludovic.courtes@inria.fr (Ludovic Courtès) writes:

> When using SA_RESTART, signal handlers are never executed, as in this
> example (checked on 2.0.9+):
>
> (sigaction SIGALRM
>   (lambda (signum)
>     (pk 'sig signum))
>   SA_RESTART)
> (alarm 3)
> (pk 'char (read-char))
>
> Presumably this is because the read(2) syscall is automatically
> restarted, leaving no chance for the handler async to run.

Thinking about this a bit -- since we always handle signals
asynchronously and have no intention of handling them synchronously,
then we just have to document this behavior.  Done in e877e1b:

  Guile handles signals asynchronously.  When it receives a signal, the
  synchronous signal handler just records the fact that a signal was
  received and sets a flag to tell the relevant Guile thread that it has a
  pending signal.  When the Guile thread checks the pending-interrupt
  flag, it will arrange to run the asynchronous part of the signal
  handler, which is the handler attached by @code{sigaction}.

  This strategy has some perhaps-unexpected interactions with the
  @code{SA_RESTART} flag, though: because the synchronous handler doesn't
  do very much, and notably it doesn't run the Guile handler, it's
  impossible to interrupt a thread stuck in a long-running system call via
  a signal handler that is installed with @code{SA_RESTART}: the
  synchronous handler just records the pending interrupt, but then the
  system call resumes and Guile doesn't have a chance to actually check
  the flag and run the asynchronous handler.  That's just how it is.

Andy

bug#14640: SA_RESTART prevents execution of signal handlers

[Ludovic Courtès]

Andy Wingo <wingo@pobox.com> skribis:

> On Mon 17 Jun 2013 15:54, ludovic.courtes@inria.fr (Ludovic Courtès) writes:
>
>> When using SA_RESTART, signal handlers are never executed, as in this
>> example (checked on 2.0.9+):
>>
>> (sigaction SIGALRM
>>   (lambda (signum)
>>     (pk 'sig signum))
>>   SA_RESTART)
>> (alarm 3)
>> (pk 'char (read-char))
>>
>> Presumably this is because the read(2) syscall is automatically
>> restarted, leaving no chance for the handler async to run.
>
> Thinking about this a bit -- since we always handle signals
> asynchronously and have no intention of handling them synchronously,
> then we just have to document this behavior.  Done in e877e1b:

I think it’s problematic though.  With the current design, signal
delivery is unreliable (with or without SA_RESTART; what we observe with
SA_RESTART occurs similarly if you make a syscall right after queuing,
but not running, an async.)

The more I think about it, the more I think a different approach is
needed.  On GNU/Linux, signalfd(2) may be part of the solution.

Ludo’.

bug#14640: SA_RESTART prevents execution of signal handlers

[Andy Wingo]

On Tue 21 Jun 2016 09:48, ludovic.courtes@inria.fr (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> On Mon 17 Jun 2013 15:54, ludovic.courtes@inria.fr (Ludovic Courtès) writes:
>>
>>> When using SA_RESTART, signal handlers are never executed, as in this
>>> example (checked on 2.0.9+):
>>>
>>> (sigaction SIGALRM
>>>   (lambda (signum)
>>>     (pk 'sig signum))
>>>   SA_RESTART)
>>> (alarm 3)
>>> (pk 'char (read-char))
>>>
>>> Presumably this is because the read(2) syscall is automatically
>>> restarted, leaving no chance for the handler async to run.
>>
>> Thinking about this a bit -- since we always handle signals
>> asynchronously and have no intention of handling them synchronously,
>> then we just have to document this behavior.  Done in e877e1b:
>
> I think it’s problematic though.  With the current design, signal
> delivery is unreliable (with or without SA_RESTART; what we observe with
> SA_RESTART occurs similarly if you make a syscall right after queuing,
> but not running, an async.)

Can you expect any kind of reasonable behavior with SA_RESTART?  I think
not.

> The more I think about it, the more I think a different approach is
> needed.  On GNU/Linux, signalfd(2) may be part of the solution.

We already do the equivalent of signalfd(), with our self-pipe trick.
And an fd doesn't help you if the syscall has no associated fd.  Signals
are just a problem.  I agree we can do better though :)

If you are just concerned about read and write, I think the right thing
is non-blocking fd's, and making the C read/write waiters also add the
signal FD to their poll set.  WDYT?

Andy

bug#14640: SA_RESTART prevents execution of signal handlers

[Ludovic Courtès]

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 21 Jun 2016 09:48, ludovic.courtes@inria.fr (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> skribis:
>>
>>> On Mon 17 Jun 2013 15:54, ludovic.courtes@inria.fr (Ludovic Courtès) writes:
>>>
>>>> When using SA_RESTART, signal handlers are never executed, as in this
>>>> example (checked on 2.0.9+):
>>>>
>>>> (sigaction SIGALRM
>>>>   (lambda (signum)
>>>>     (pk 'sig signum))
>>>>   SA_RESTART)
>>>> (alarm 3)
>>>> (pk 'char (read-char))
>>>>
>>>> Presumably this is because the read(2) syscall is automatically
>>>> restarted, leaving no chance for the handler async to run.
>>>
>>> Thinking about this a bit -- since we always handle signals
>>> asynchronously and have no intention of handling them synchronously,
>>> then we just have to document this behavior.  Done in e877e1b:
>>
>> I think it’s problematic though.  With the current design, signal
>> delivery is unreliable (with or without SA_RESTART; what we observe with
>> SA_RESTART occurs similarly if you make a syscall right after queuing,
>> but not running, an async.)
>
> Can you expect any kind of reasonable behavior with SA_RESTART?  I think
> not.

In C it does its job: signal handlers run, and other parts of the code
don’t notice the interruption.  Here it’s useless.

>> The more I think about it, the more I think a different approach is
>> needed.  On GNU/Linux, signalfd(2) may be part of the solution.
>
> We already do the equivalent of signalfd(), with our self-pipe trick.

Hmm, I wonder if it’s really equivalent.

Also, it is internal: the Scheme level cannot explicitly “convert”
signals to FDs; all it gets is those asyncs.

> And an fd doesn't help you if the syscall has no associated fd.

But those typically don’t block.

> If you are just concerned about read and write, I think the right thing
> is non-blocking fd's, and making the C read/write waiters also add the
> signal FD to their poll set.  WDYT?

There’s also ‘select’.  In the case of the Shepherd, you’re in a
‘select’ loop, waiting for events from file descriptors *and* waiting
for SIGCHLD.  However, the SIGCHLD handler can end up running long after
the fact.  In this case, it would help to explicitly use use ‘signalfd’
at the Scheme level (whether it’s used internally in libguile doesn’t
matter.)

Not sure I follow your suggestion.  My naïve view is that one would
probably expect/want signals to behave “like in C”, meaning that
handlers would run in a timely fashion once the signal has effectively
been received by libguile.

Thanks for your feedback!

Ludo’.