* bug#6074: 24.0.50; accept-process-output on listening sockets not incorruptible
@ 2010-05-01 22:15 Helmut Eller
2019-07-22 3:52 ` bug#6074: accept-process-output on listening sockets cause non-interruptible infloop Pip Cet
0 siblings, 1 reply; 8+ messages in thread
From: Helmut Eller @ 2010-05-01 22:15 UTC (permalink / raw
To: 6074
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))
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2010-05-01 22:15 bug#6074: 24.0.50; accept-process-output on listening sockets not incorruptible Helmut Eller
@ 2019-07-22 3:52 ` Pip Cet
2019-07-22 14:26 ` Eli Zaretskii
2019-07-23 2:46 ` Richard Stallman
0 siblings, 2 replies; 8+ messages in thread
From: Pip Cet @ 2019-07-22 3:52 UTC (permalink / raw
To: 6074
[-- Attachment #1: Type: text/plain, Size: 946 bytes --]
> 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.
[-- Attachment #2: 0001-Don-t-retry-reading-after-receiving-EINVAL-bug-6074.patch --]
[-- Type: text/x-patch, Size: 849 bytes --]
From ad294bb1e7ff7012dca3345a8ca2046e0f9dc8fb Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Mon, 22 Jul 2019 03:49:54 +0000
Subject: [PATCH] Don't retry reading after receiving EINVAL (bug#6074)
* src/process.c (wait_reading_process_output): Don't retry reading
from an fd after an unknown error.
---
src/process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
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
--
2.22.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-22 3:52 ` bug#6074: accept-process-output on listening sockets cause non-interruptible infloop Pip Cet
@ 2019-07-22 14:26 ` Eli Zaretskii
2019-07-22 17:45 ` Pip Cet
2019-07-23 2:46 ` Richard Stallman
1 sibling, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2019-07-22 14:26 UTC (permalink / raw
To: Pip Cet; +Cc: 6074
> From: Pip Cet <pipcet@gmail.com>
> 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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-22 14:26 ` Eli Zaretskii
@ 2019-07-22 17:45 ` Pip Cet
2020-09-14 13:21 ` Lars Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Pip Cet @ 2019-07-22 17:45 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 6074
[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]
On Mon, Jul 22, 2019 at 2:26 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Pip Cet <pipcet@gmail.com>
> > 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?
[-- Attachment #2: 0001-Don-t-retry-reading-after-receiving-EINVAL-bug-6074.patch --]
[-- Type: text/x-patch, Size: 1060 bytes --]
From a8991ccc1e846aed9a8f1ba0ebcd2771b623f241 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Mon, 22 Jul 2019 03:49:54 +0000
Subject: [PATCH] Don't retry reading after receiving EINVAL (bug#6074)
* src/process.c (wait_reading_process_output): Don't retry reading
from an fd after an unknown error.
---
src/process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/process.c b/src/process.c
index abadabe77e..9104a392c8 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5270,14 +5270,16 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
/* If data can be read from the process, do so until exhausted. */
if (wait_proc->infd >= 0)
{
+ 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
--
2.22.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-22 3:52 ` bug#6074: accept-process-output on listening sockets cause non-interruptible infloop Pip Cet
2019-07-22 14:26 ` Eli Zaretskii
@ 2019-07-23 2:46 ` Richard Stallman
2019-07-23 14:31 ` Pip Cet
1 sibling, 1 reply; 8+ messages in thread
From: Richard Stallman @ 2019-07-23 2:46 UTC (permalink / raw
To: Pip Cet; +Cc: 6074
[[[ 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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-23 2:46 ` Richard Stallman
@ 2019-07-23 14:31 ` Pip Cet
2019-07-25 3:08 ` Richard Stallman
0 siblings, 1 reply; 8+ messages in thread
From: Pip Cet @ 2019-07-23 14:31 UTC (permalink / raw
To: rms; +Cc: 6074
On Tue, Jul 23, 2019 at 2:46 AM Richard Stallman <rms@gnu.org> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-23 14:31 ` Pip Cet
@ 2019-07-25 3:08 ` Richard Stallman
0 siblings, 0 replies; 8+ messages in thread
From: Richard Stallman @ 2019-07-25 3:08 UTC (permalink / raw
To: Pip Cet; +Cc: 6074
[[[ 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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#6074: accept-process-output on listening sockets cause non-interruptible infloop
2019-07-22 17:45 ` Pip Cet
@ 2020-09-14 13:21 ` Lars Ingebrigtsen
0 siblings, 0 replies; 8+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-14 13:21 UTC (permalink / raw
To: Pip Cet; +Cc: 6074
Pip Cet <pipcet@gmail.com> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-14 13:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-01 22:15 bug#6074: 24.0.50; accept-process-output on listening sockets not incorruptible Helmut Eller
2019-07-22 3:52 ` bug#6074: accept-process-output on listening sockets cause non-interruptible infloop Pip Cet
2019-07-22 14:26 ` Eli Zaretskii
2019-07-22 17:45 ` Pip Cet
2020-09-14 13:21 ` Lars Ingebrigtsen
2019-07-23 2:46 ` Richard Stallman
2019-07-23 14:31 ` Pip Cet
2019-07-25 3:08 ` Richard Stallman
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.