Re: SCM_SYSCALL

[ludo@gnu.org (Ludovic Courtès)]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> We have this (since 2010):
>>
>> #   define SCM_SYSCALL(line)                    \
>>   do                                            \
>>     {                                           \
>>       errno = 0;                                \
>>       line;                                     \
>>       if (errno == EINTR)                       \
>>         {                                       \
>>           SCM_ASYNC_TICK;                       \
>>           continue;                             \
>>         }                                       \
>>     }                                           \
>>   while(0)
>>
>> It turns out that the effect upon EINTR is to leave the loop.  So
>> typically, fport_fill_input just throws to system-error and reveals the
>> EINTR, contrary to SCM_SYSCALL intends to do.
>
> Ugh.  Well, I guess this finally explains <http://bugs.gnu.org/13018>.

Indeed.  (Funny to see how I was blissfully quoting the above macro
saying: “look, EINTR is handled, of course!”.  :-))

> I strongly believe that we should fix this in stable-2.0.  While it is
> true that the above scenario is possible, I suspect it is at least an
> order of magnitude more common for Guile-based software to be written
> based on the presumption that EINTR is handled automatically.
>
> Not only did all versions of Guile 1.x automatically handle EINTR, but
> most of us assumed that this behavior was unchanged in Guile 2.0 and
> wrote our software based on that assumption.  I certainly did.
>
> As it is now, even portable Scheme code that uses (read) might result in
> exceptions being thrown semi-randomly.  We cannot reasonably expect
> Guile programs to put each (read) within a loop to handle EINTR.
>
> Please, let's fix this in stable-2.0.

Yes, I’ve reached that conclusion too.

I’ve been cooking a patch but the test case ends up being trickier to
write than I expected.  Here’s what I have:

        (let* ((in+out   (pk 'pipe (pipe)))
               (lock     (make-mutex))
               (cond     (make-condition-variable))
               (signaled #f)
               (thread   (call-with-new-thread
                          (lambda ()
                            (with-mutex lock
                              (display "hello " (cdr in+out))
                              (wait-condition-variable cond lock)
                              (display "world\n" (cdr in+out))
                              (close-port (cdr in+out)))))))
          (define handle
            (lambda (signum)
              (with-mutex lock
                (set! signaled (pk 'sig signum))
                (signal-condition-variable cond))))
          (sigaction SIGALRM handle 0)
          (alarm 2)

          ;; This thread (the main thread) receives the signal.  Yet,
          ;; the EINTR returned by read(2) as called via `read-line'
          ;; must be swallowed.
          (let ((line (read-line (car in+out))))
            (join-thread thread)
            (list signaled line)))

This nicely reproduces the problem where fport_fill_input throws to
‘system-error’ with EINTR.

However, with a fixed SCM_SYSCALL, the result is pretty much the same as
with SA_RESTART (see <http://bugs.gnu.org/14640>): when SCM_ASYNC_TICK
is called right after we get EINTR, chances are that the async hasn’t
been queued yet, so we get back to our read(2) call, and thus the
Scheme-level signal handler is never called.  (Typically, when running
the test through strace, it passes, because the timing is “better”, but
it fails without strace.)

Suggestions?

Thanks,
Ludo’.