unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Paul Eggert <eggert@cs.ucla.edu>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Michael Albinus <michael.albinus@gmx.de>, emacs-devel@gnu.org
Subject: Re: some accept-process-output races fixed; Tramp FIXMEs
Date: Wed, 16 Jan 2019 10:39:45 -0800	[thread overview]
Message-ID: <80d86b5a-efb7-87cc-a3db-0a6ad356274d@cs.ucla.edu> (raw)
In-Reply-To: <jwvr2dcwx9b.fsf-monnier+gmane.emacs.devel@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

On 1/16/19 5:25 AM, Stefan Monnier wrote:
 > So how can the code tell when it has read all the data?

The code keeps calling accept-process-output until it gets nil.

Now that I think about it, if you just want all the data you needn’t 
call process-live-p. I installed the attached doc patch accordingly.

The Tramp code still contains several loops like this:

   (while (or (accept-process-output p 0.1)
              (process-live-p p)))

that suffer from race conditions. Consider the following sequence of events:

* accept-process-output times out after 0.1 seconds, and returns nil.
* P generates some data and then exits.
* process-live-p returns nil.

In this case the loop will exit and lose data. This bug is caused by the 
" 0.1" in that loop. I don’t know why the " 0.1" is there, but if the " 
0.1" has to be there then I suppose one way to fix the bug would be to 
enhance accept-process-output so that its caller can distinguish a 
timeout from a connection-closed.

[-- Attachment #2: 0001-doc-lispref-processes.texi-Accepting-Output-Simplify.patch --]
[-- Type: text/x-patch, Size: 1176 bytes --]

From b76e90d242e9ee0a733fc9d4b6e280eb6892a6d0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 16 Jan 2019 10:31:21 -0800
Subject: [PATCH] * doc/lispref/processes.texi (Accepting Output): Simplify.

---
 doc/lispref/processes.texi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index afda8aede8..fd6686e882 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -1870,13 +1870,13 @@ Accepting Output
 @end example
 
 @noindent
-will often work, it has a race condition and can miss some output if
-@code{process-live-p} returns @code{nil} while the connection still
-contains data.  Better is to write the loop like this:
+will often read all output from @var{process}, it has a race condition
+and can miss some output if @code{process-live-p} returns @code{nil}
+while the connection still contains data.  Better is to write the loop
+like this:
 
 @example
-(while (or (accept-process-output process)
-           (process-live-p process)))
+(while (accept-process-output process))
 @end example
 
 @node Processes and Threads
-- 
2.20.1


  reply	other threads:[~2019-01-16 18:39 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-15 18:33 some accept-process-output races fixed; Tramp FIXMEs Paul Eggert
2019-01-16  8:32 ` Michael Albinus
2019-01-16  8:38   ` Paul Eggert
2019-01-16  8:49     ` Michael Albinus
2019-01-16 13:03   ` Michael Albinus
2019-01-16 13:25 ` Stefan Monnier
2019-01-16 18:39   ` Paul Eggert [this message]
2019-01-16 19:07     ` Stefan Monnier
2019-01-17  0:15       ` Paul Eggert
2019-01-20  9:57     ` Michael Albinus
2019-01-21 23:02       ` Daniel Colascione
2019-01-22 16:54         ` Eli Zaretskii
2019-01-22  7:09       ` Paul Eggert
2019-01-22 16:56       ` Eli Zaretskii
2019-01-22 18:58         ` Michael Albinus
2019-01-22 22:06       ` Stefan Monnier
2019-01-22 22:45         ` Michael Albinus
2019-01-23 16:18           ` Eli Zaretskii
2019-01-23 18:35             ` Michael Albinus
2019-01-23 22:19               ` Daniel Pittman
2019-01-23 16:47           ` Stefan Monnier
2019-01-23 17:36             ` Paul Eggert
2019-01-28 15:37               ` Michael Albinus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80d86b5a-efb7-87cc-a3db-0a6ad356274d@cs.ucla.edu \
    --to=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    --cc=michael.albinus@gmx.de \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).