bug#6074: 24.0.50; accept-process-output on listening sockets not incorruptible

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#6074: 24.0.50; accept-process-output on listening sockets not incorruptible

Helmut Eller-2
With this code Emacs seems to be stuck in an endless loop and is not
incorruptible with C-g:

(let ((proc (make-network-process :name "foo" :server t :noquery t
                                  :family 'local :service "/tmp/foo.socket")))
  (accept-process-output proc))





Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Pip Cet
> With this code Emacs seems to be stuck in an endless loop and is not
> incorruptible with C-g:

> (let ((proc (make-network-process :name "foo" :server t :noquery t
>  :family 'local :service "/tmp/foo.socket")))
>   (accept-process-output proc))

On Linux, the problem appears to be that we don't abort this infloop
in process.c if a read () returns EINVAL:

          while (true)
        {
          int nread = read_process_output (proc, wait_proc->infd);
          if (nread < 0)
            {
              if (errno == EIO || would_block (errno))
            break;
            }
          else
            {
              if (got_some_output < nread)
            got_some_output = nread;
              if (nread == 0)
            break;
              read_some_bytes = true;
            }
        }

That seems problematic to me, since we might get non-EIO errors for
other reasons. I'm attaching a patch that appears to fix the issue.

0001-Don-t-retry-reading-after-receiving-EINVAL-bug-6074.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Eli Zaretskii
> From: Pip Cet <[hidden email]>
> Date: Mon, 22 Jul 2019 03:52:33 +0000
>
> diff --git a/src/process.c b/src/process.c
> index abadabe77e..1311409274 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -5277,7 +5277,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
>    int nread = read_process_output (proc, wait_proc->infd);
>    if (nread < 0)
>      {
> -      if (errno == EIO || would_block (errno))
> +      if (errno == EINTR)
> + continue;
> +      else
>   break;
>      }
>    else

Isn't it better to simply call rarely_quit inside the loop?



Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Pip Cet
On Mon, Jul 22, 2019 at 2:26 PM Eli Zaretskii <[hidden email]> wrote:

>
> > From: Pip Cet <[hidden email]>
> > Date: Mon, 22 Jul 2019 03:52:33 +0000
> >
> > diff --git a/src/process.c b/src/process.c
> > index abadabe77e..1311409274 100644
> > --- a/src/process.c
> > +++ b/src/process.c
> > @@ -5277,7 +5277,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
> >                 int nread = read_process_output (proc, wait_proc->infd);
> >                 if (nread < 0)
> >                   {
> > -                   if (errno == EIO || would_block (errno))
> > +                   if (errno == EINTR)
> > +                     continue;
> > +                   else
> >                       break;
> >                   }
> >                 else
>
> Isn't it better to simply call rarely_quit inside the loop?
Why would we try again after receiving EINVAL? I believe the right
behavior is to return immediately in this case, just as we do for EIO.

However, we should probably call rarely_quit inside the loop, anyway,
to catch the case of a kernel bug keeping us busy with EINTRs. How's
this?

0001-Don-t-retry-reading-after-receiving-EINVAL-bug-6074.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Richard Stallman
In reply to this post by Pip Cet
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > On Linux, the problem appears to be that

Those words lead me to think that you're running GNU Emacs as
part of the GNU operating system -- a version with Linx as the kernel.
One of the obstacles that we GNU developers have to face
is that people that use GNU don't know it is GNU.  They think
the system is "Linux".

Thanks for reporting a bug in one part of the GNU system.  While
you're at it, could you please call the system "GNU/Linux", so as to
help correct the misinformation of what it is, where it comes from,
and why we developed it?

See https://gnu.org/gnu/linux-and-gnu.html and
https://gnu.org/gnu/gnu-linux-faq.html, plus the history in
https://gnu.org/gnu/the-gnu-project.html.


--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Pip Cet
On Tue, Jul 23, 2019 at 2:46 AM Richard Stallman <[hidden email]> wrote:
>   > On Linux, the problem appears to be that
>
> Those words lead me to think that you're running GNU Emacs as
> part of the GNU operating system -- a version with Linx as the kernel.

I am, indeed, but in this particular case, it is the Linux kernel
doing something problematic: returning -EINVAL, which glibc merely
stores in errno.  But I agree it would have been more accurate to say
something like "On this GNU/Linux system, the problem appears to be
that the Linux kernel..."

> Thanks for reporting a bug in one part of the GNU system.  While

(I didn't report it, I merely reproduced a very old bug report and
suggested a fix.)

> you're at it, could you please call the system "GNU/Linux", so as to
> help correct the misinformation of what it is, where it comes from,
> and why we developed it?

Thanks for taking the time to remind me of that.



Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Richard Stallman
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Happy hacking!

--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





Reply | Threaded
Open this post in threaded view
|

bug#6074: accept-process-output on listening sockets cause non-interruptible infloop

Lars Ingebrigtsen
In reply to this post by Pip Cet
Pip Cet <[hidden email]> writes:

> Why would we try again after receiving EINVAL? I believe the right
> behavior is to return immediately in this case, just as we do for EIO.
>
> However, we should probably call rarely_quit inside the loop, anyway,
> to catch the case of a kernel bug keeping us busy with EINTRs. How's
> this?

[...]

>      {
> +      unsigned int count = 0;
>        XSETPROCESS (proc, wait_proc);
>  
>        while (true)
>   {
>    int nread = read_process_output (proc, wait_proc->infd);
> +  rarely_quit (++count);
>    if (nread < 0)
>      {
> -      if (errno == EIO || would_block (errno))
> +      if (errno != EINTR)
>   break;
>      }
>    else

There was no followup on this patch at the time (except Richard chiding
you for using forbidden terminology), but I tried the patch now, and it
seems to fix the issue, so I've applied it to the trunk now.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no