emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[debbugs-tracker] bug#20978: closed (25.0.50; [PATCH 0/7] Emacs can retu


From: GNU bug Tracking System
Subject: [debbugs-tracker] bug#20978: closed (25.0.50; [PATCH 0/7] Emacs can return too fast when reading from any processes)
Date: Mon, 06 Jul 2015 02:30:08 +0000

Your message dated Sun, 05 Jul 2015 19:28:54 -0700
with message-id <address@hidden>
and subject line Re: 25.0.50; [PATCH 0/7] Emacs can return too fast when 
reading from any processes
has caused the debbugs.gnu.org bug report #20978,
regarding 25.0.50; [PATCH 0/7] Emacs can return too fast when reading from any 
processes
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden)


-- 
20978: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20978
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- 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



--- End Message ---
--- Begin Message --- 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 User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 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.

Attachment: 0008-Avoid-duplicate-calls-to-current_timespec.patch
Description: Text Data


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]