GNU bug report logs - #20978
25.0.50; [PATCH 0/7] Emacs can return too fast when reading from any processes

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Ian Kelling <ian <at> iankelling.org>
Subject: bug#20978: closed (Re: 25.0.50; [PATCH 0/7] Emacs can return too
 fast when reading from any processes)
Date: Mon, 06 Jul 2015 02:30:11 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#20978: 25.0.50; [PATCH 0/7] Emacs can return too fast when reading from any processes

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 20978 <at> debbugs.gnu.org.

-- 
20978: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20978
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Ian Kelling <ian <at> iankelling.org>
Cc: 20978-done <at> debbugs.gnu.org
Subject: Re: 25.0.50; [PATCH 0/7] Emacs can return too fast when reading from
 any processes
Date: Sun, 05 Jul 2015 19:28:54 -0700

[Message part 3 (text/plain, inline)]

Thanks for the patches; they were very clear and organized, in a difficult code 
area.  I installed them into the Emacs master with minor changes, along with the 
attached further patch, and am marking this bug as done.

As far as patch format goes, I mildly prefer attachments but the form you used 
was fine.  My preference is because I am not on the bug-gnu-emacs mailing list, 
and don't know how to retrieve your email from debbugs.gnu.org or from 
lists.gnu.org -- the best I could come up with was the mbox format of 
lists.gnu.org but that did not apply cleanly.

[0008-Avoid-duplicate-calls-to-current_timespec.patch (text/x-diff, attachment)]

[Message part 5 (message/rfc822, inline)]

From: Ian Kelling <ian <at> iankelling.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50;
 [PATCH 0/7] Emacs can return too fast when reading from any processes
Date: Sat, 04 Jul 2015 05:34:33 -0700

In GNU Emacs 25.0.50.63 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2015-07-04
Repository revision: 5c36788e76b22587e554960ed837f724473597a9
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04.2 LTS

Configured using:
 `configure 'CFLAGS=-O0 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11


Overview:

The patch for debbugs:17647 causes emacs to return too fast sometimes
when reading from any processes. It made adaptive read buffering be
mostly dead code and caused readline-complete package to break. Instead,
limit timeout less aggressively.

Reproducing the problem:

emacs -Q -l repro.el

;; repro.el
(progn
  (setq explicit-bash-args '("--norc"))
  (setenv "EMACS" "")
  (setq comint-process-echoes t)
  (shell)
  (let* ((proc (get-buffer-process (current-buffer)))
         (filter-orig (process-filter proc))
         accumulated-output)
    (process-send-string proc "stty echo\n")
    (set-process-query-on-exit-flag proc nil)
    (dotimes (unused 10)
      (sleep-for .05))
    (set-process-filter proc (lambda (proc string) (push (list string) accumulated-output)))
    (process-send-string proc "echo a bit of text for testing\n")
    (dotimes (unused 100)
      (sleep-for .03))
    (message "waiting for any process: %s" (reverse accumulated-output))
    (switch-to-buffer "*Messages*")
    nil))


Output without my patches:

waiting for any process:
((ech) (o a ) (bi) (t ) (o) (f ) (t) (e) (x) (t) ( ) (f) (o) (r) ( ) (t) (e) (s) (t) (i) (n) (g) (
) (a bit of text for testing
) (bash-4.3$ ))

Output with my patches (also the same same output as before the
debbugs:17647 patch, aka git 05d2821):

waiting for any process:
((ec) (ho a bit of text for testing
a bit of text for testing
bash-4.3$ ))


Background:

The 17647 patch had a few things bundled in and the root problem and fix
was not documented. The problem was that we got output, and we
subsequently got a pselect of 0, but we have set
timeout_reduced_for_timers, so we don't break when we should. This is
the relevant condition:

if ((time_limit || nsecs) && nfds == 0 && ! timeout_reduced_for_timers)
  break;

This was fixed by setting the timeout to 0 if we've attempted to read
process output. However, that was too big a hammer.

What we actually want is what the original code tried to do, but had
some logic problems: read data, and stop if there is no more to read in
a reasonable amount of time or we've reached the requested timeout.

There is a reference to someone else having this issue as well, without
figuring out anything about it
http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-06/msg00958.html
And there is a resulting workaround in flymake-tests.el.


About the patches:

The first 4 are refactoring and not necessarily dependent on any others,
but independent patches would conflict and they make the code clearer so
I included them. I can separate any of them if needed.

Patch 3 depends on the patch for debbugs:20976, which is a simple fix
and I expect no problem for it be applied first.

I was careful to not squash unrelated changes as the function is rather
complicated and I could have saved a fair amount of time if there
weren't unrelated changes in the patch that introduced this bug.

The last 2 patches are probably easier to understand squashed together,
but the last one one fixes a different avenue for the same bug to
manifest (SIGIO), which I haven't tried to reproduce, and includes more
circumstances (reading from a specific process and before we've gotten
any output), and I'm not sure if some code might depend on it's behavior
so I broke it out.

Make check succeeds on all the patches.

Ian Kelling (7):
  ; Minor cleanup of wait_reading_process_output
  ; Remove ADAPTIVE_READ_BUFFERING ifdef
  ; Rename local var to match function name
  ; Rename local var nsecs to adaptive_nsecs
  : Refactor timeouts in wait_reading_process_output
  Don't return as fast reading any process output
  Avoid returning early reading process output due to SIGIO

 src/process.c | 211 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 98 insertions(+), 113 deletions(-)

--
2.4.5

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.