bug#13018: fport_fill_input should handle EINTR

bug#13018: fport_fill_input should handle EINTR

[Aidan Gauland]

Guile version: 3.2.0-4-amd64
OS: Debian wheezy GNU/Linux, kernel 3.2.0-4-amd64

In a program that maintains a TCP connection with a polling loop, I have
defined a signal handler with the `sigaction' procedure to terminate the
connection gracefully.  When the trapped signal is received (in this
case SIGINT) and the handler called, the following error message is
printed (full backtrace omitted):

ERROR: In procedure %read-line:
ERROR: In procedure fport_fill_input: Interrupted system call

I mentioned this on #guile on freenode and mark_weaver informed me that
I should be setting the SA_RESTART flag to avoid this problem, but that
there is a bug in Guile causing the error.

<mark_weaver> fport_fill_input should handle an EINTR error from 'read',
              and restart the read if that happens.

<mark_weaver> by default on some systems, signals cause 'read', 'write',
              and many other system calls to abort and return an EINTR
              error.

<mark_weaver> basically, at the POSIX level, every call to 'read' has to
              be within a little loop that takes care of the EINTR
              problem.

In short, I don't fully understand the innards to have an opinion on
whether this a bug or users should just set the SA_RESTART flag, bug
Mark assures me this is, indeed, a bug.  Any disagreement, take it up
with him. ;)

Regards,
Aidan Gauland

bug#13018: fport_fill_input should handle EINTR

[Ludovic Courtès]

Hi!

Aidan Gauland <aidalgol@no8wireless.co.nz> skribis:

> <mark_weaver> fport_fill_input should handle an EINTR error from 'read',
>               and restart the read if that happens.
>
> <mark_weaver> by default on some systems, signals cause 'read', 'write',
>               and many other system calls to abort and return an EINTR
>               error.
>
> <mark_weaver> basically, at the POSIX level, every call to 'read' has to
>               be within a little loop that takes care of the EINTR
>               problem.

‘fport_fill_input’ does this:

  SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));

and SCM_SYSCALL does that:

  # ifdef EINTR
  #  if (EINTR > 0)
  #   define SCM_SYSCALL(line)                    \
    do                                            \
      {                                           \
        errno = 0;                                \
        line;                                     \
        if (errno == EINTR)                       \
          {                                       \
            SCM_ASYNC_TICK;                       \
            continue;                             \
          }                                       \
      }                                           \
    while(0)
  #  endif /*  (EINTR > 0) */
  # endif /* def EINTR */

On GNU/Linux, I see:

  $ echo '#include <errno.h>' | gcc -E -dM - | grep EINTR
  #define EINTR 4

So AFAICS, the EINTR case is taken care of.  Or am I missing something?

Do you have a reduced test case?

Thanks,
Ludo’.

bug#13018: fport_fill_input should handle EINTR

[Andy Wingo]

On Thu 29 Nov 2012 21:19, ludo@gnu.org (Ludovic Courtès) writes:

> Aidan Gauland <aidalgol@no8wireless.co.nz> skribis:
>
>> <mark_weaver> fport_fill_input should handle an EINTR error from 'read',
>>               and restart the read if that happens.
>>
>> <mark_weaver> by default on some systems, signals cause 'read', 'write',
>>               and many other system calls to abort and return an EINTR
>>               error.
>>
>> <mark_weaver> basically, at the POSIX level, every call to 'read' has to
>>               be within a little loop that takes care of the EINTR
>>               problem.
>
> ‘fport_fill_input’ does this:
>
>   SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
>
> and SCM_SYSCALL does that:
>
>   # ifdef EINTR
>   #  if (EINTR > 0)
>   #   define SCM_SYSCALL(line)                    \
>     do                                            \
>       {                                           \
>         errno = 0;                                \
>         line;                                     \
>         if (errno == EINTR)                       \
>           {                                       \
>             SCM_ASYNC_TICK;                       \
>             continue;                             \
>           }                                       \
>       }                                           \
>     while(0)
>   #  endif /*  (EINTR > 0) */
>   # endif /* def EINTR */

I get the feeling the EINTR is coming from somewhere else -- like the
ASYNC_TICK.  The sigaction could cause a thunk to run there.

Aidan, do you have a test case?

Andy
-- 
http://wingolog.org/

bug#13018: fport_fill_input should handle EINTR

[Aidan Gauland]

> Aidan, do you have a test case?

I haven't been able to reproduce this with a minimal example, but I can
consistently reproduce it with my IRC bot
<https://github.com/aidalgol/cunning-bot>.

Replace the last two lines (lines 22 and 23) of run-cbot.scm with...

(define bot (make-bot "Cunning_Bot" "Cunning_Bot" "Cunning Bot" "chat.freenode.net" 6667))
(sigaction SIGINT
  (lambda ()
    (disconnect-bot bot)))
(start-bot bot '("#cunningbot"))

...run it (run-cbot.scm), and after it says

Setting up IRC connection...done.
Joining channels...done.

type ^C

I get this backtrace:

^CBacktrace:
In ice-9/boot-9.scm:
 157: 12 [catch #t #<catch-closure 9a1020> ...]
In unknown file:
   ?: 11 [apply-smob/1 #<catch-closure 9a1020>]
In ice-9/boot-9.scm:
  63: 10 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 421: 9 [eval # #]
In ice-9/boot-9.scm:
2131: 8 [save-module-excursion #<procedure 9a01c0 at ice-9/boot-9.scm:3711:3 ()>]
3718: 7 [#<procedure 9a01c0 at ice-9/boot-9.scm:3711:3 ()>]
In unknown file:
   ?: 6 [load-compiled/vm "/home/aidan/.cache/guile/ccache/2.0-LE-8-2.0/home/aidan/src/cunning-bot/run-cbot.scm.go"]
In /home/aidan/src/cunning-bot/bot.scm:
 260: 5 [start-bot # #]
 115: 4 [read-line-irc #]
In ice-9/rdelim.scm:
 129: 3 [read-line #<input-output: socket 11> trim]
In unknown file:
   ?: 2 [%read-line #<input-output: socket 11>]
In ice-9/boot-9.scm:
 184: 1 [throw system-error "fport_fill_input" "~A" ("Interrupted system call") (4)]
In /home/aidan/src/cunning-bot/run-cbot.scm:
  24: 0 [#<procedure 12007a0 at /home/aidan/src/cunning-bot/run-cbot.scm:24:2 ()> 2]

/home/aidan/src/cunning-bot/run-cbot.scm:24:2: In procedure #<procedure 12007a0 at /home/aidan/src/cunning-bot/run-cbot.scm:24:2 ()>:
/home/aidan/src/cunning-bot/run-cbot.scm:24:2: Wrong number of arguments to #<procedure 12007a0 at /home/aidan/src/cunning-bot/run-cbot.scm:24:2 ()>

Could this be caused by a bug in spells?

I wish I could give a simpler test case. :(

Regards,
Aidan Gauland

(Sorry I forgot to group reply the first time.)

bug#13018: fport_fill_input should handle EINTR

[Mark H Weaver]

We now finally understand the cause of this bug.  See below.

Andy Wingo <wingo@pobox.com> writes:
> On Thu 29 Nov 2012 21:19, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Aidan Gauland <aidalgol@no8wireless.co.nz> skribis:
>>
>>> <mark_weaver> fport_fill_input should handle an EINTR error from 'read',
>>>               and restart the read if that happens.
>>>
>>> <mark_weaver> by default on some systems, signals cause 'read', 'write',
>>>               and many other system calls to abort and return an EINTR
>>>               error.
>>>
>>> <mark_weaver> basically, at the POSIX level, every call to 'read' has to
>>>               be within a little loop that takes care of the EINTR
>>>               problem.
>>
>> ‘fport_fill_input’ does this:
>>
>>   SCM_SYSCALL (count = read (fp->fdes, pt->read_buf, pt->read_buf_size));
>>
>> and SCM_SYSCALL does that:
>>
>>   # ifdef EINTR
>>   #  if (EINTR > 0)
>>   #   define SCM_SYSCALL(line)                    \
>>     do                                            \
>>       {                                           \
>>         errno = 0;                                \
>>         line;                                     \
>>         if (errno == EINTR)                       \
>>           {                                       \
>>             SCM_ASYNC_TICK;                       \
>>             continue;                             \
>>           }                                       \
>>       }                                           \
>>     while(0)
>>   #  endif /*  (EINTR > 0) */
>>   # endif /* def EINTR */
>
> I get the feeling the EINTR is coming from somewhere else [...]

As Ludovic finally realized, the 'continue' above always exits the loop,
because 'continue' jumps to the test, rather than unconditionally
jumping to the top of the loop body.  This bug was introduced in Guile
2.0.0.  We are now discussing how to proceed on guile-devel:

http://lists.gnu.org/archive/html/guile-devel/2013-06/msg00050.html
http://lists.gnu.org/archive/html/guile-devel/2013-07/msg00002.html

    Thanks,
      Mark

bug#13018: fport_fill_input should handle EINTR

[Mark H Weaver]

This bug was fixed in fe51c7b3e0a1e93be3bb81dd2d4b18936fe2df3a.

    Thanks!
      Mark

bug#13018: fport_fill_input should handle EINTR

[Andy Wingo]

On Wed 28 Nov 2012 09:19, Aidan Gauland <aidalgol@no8wireless.co.nz> writes:

> In a program that maintains a TCP connection with a polling loop, I have
> defined a signal handler with the `sigaction' procedure to terminate the
> connection gracefully.  When the trapped signal is received (in this
> case SIGINT) and the handler called, the following error message is
> printed (full backtrace omitted):
>
> ERROR: In procedure %read-line:
> ERROR: In procedure fport_fill_input: Interrupted system call

Any more info here?

Andy
-- 
http://wingolog.org/