From: Paul Eggert <eggert@cs.ucla.edu>
To: Chiara Orsini <chiara.orsini@gmail.com>
Cc: David Reitter <david.reitter@gmail.com>, 17805@debbugs.gnu.org
Subject: bug#17805: 24.3.50; Crash in -[EmacsApp / Faccept_process_output
Date: Wed, 18 Jun 2014 14:36:10 -0700 [thread overview]
Message-ID: <53A2064A.6030903@cs.ucla.edu> (raw)
In-Reply-To: <42271636-7A61-40B0-919C-67313B986EB4@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 146 bytes --]
Thanks for the bug report. Is the bug reproducible? Does the attached
patch fix it? Sorry, I don't use OS X, so I can't easily test this fix.
[-- Attachment #2: 17805.diff --]
[-- Type: text/plain, Size: 10539 bytes --]
=== modified file 'src/ChangeLog'
--- src/ChangeLog 2014-06-18 19:17:23 +0000
+++ src/ChangeLog 2014-06-18 21:32:40 +0000
@@ -1,3 +1,16 @@
+2014-06-18 Paul Eggert <eggert@cs.ucla.edu>
+
+ Avoid hangs in accept-process-output (Bug#17647, Bug#17805).
+ This fix is backported from the trunk.
+ * lisp.h, process.c (wait_reading_process_input):
+ Return int, not bool. All uses changed.
+ * process.c (SELECT_CANT_DO_WRITE_MASK):
+ Remove macro, replacing with ...
+ (SELECT_CAN_DO_WRITE_MASK): ... new constant, with inverted sense.
+ All uses changed.
+ (status_notify): New arg WAIT_PROC. Return int, not void.
+ All uses changed.
+
2014-06-18 Eli Zaretskii <eliz@gnu.org>
* image.c [5 < GIFLIB_MAJOR + (1 <= GIFLIB_MINOR)]: Declare the
=== modified file 'src/lisp.h'
--- src/lisp.h 2014-06-01 07:06:28 +0000
+++ src/lisp.h 2014-06-18 21:26:42 +0000
@@ -4187,10 +4187,8 @@
/* Defined in process.c. */
extern Lisp_Object QCtype, Qlocal;
extern void kill_buffer_processes (Lisp_Object);
-extern bool wait_reading_process_output (intmax_t, int, int, bool,
- Lisp_Object,
- struct Lisp_Process *,
- int);
+extern int wait_reading_process_output (intmax_t, int, int, bool, Lisp_Object,
+ struct Lisp_Process *, int);
/* Max value for the first argument of wait_reading_process_output. */
#if __GNUC__ == 3 || (__GNUC__ == 4 && __GNUC_MINOR__ <= 5)
/* Work around a bug in GCC 3.4.2, known to be fixed in GCC 4.6.3.
=== modified file 'src/process.c'
--- src/process.c 2014-05-03 20:13:10 +0000
+++ src/process.c 2014-06-18 21:27:30 +0000
@@ -224,8 +224,9 @@
/* Only W32 has this, it really means that select can't take write mask. */
#ifdef BROKEN_NON_BLOCKING_CONNECT
#undef NON_BLOCKING_CONNECT
-#define SELECT_CANT_DO_WRITE_MASK
+enum { SELECT_CAN_DO_WRITE_MASK = false };
#else
+enum { SELECT_CAN_DO_WRITE_MASK = true };
#ifndef NON_BLOCKING_CONNECT
#ifdef HAVE_SELECT
#if defined (HAVE_GETPEERNAME) || defined (GNU_LINUX)
@@ -281,7 +282,7 @@
static bool keyboard_bit_set (fd_set *);
#endif
static void deactivate_process (Lisp_Object);
-static void status_notify (struct Lisp_Process *);
+static int status_notify (struct Lisp_Process *, struct Lisp_Process *);
static int read_process_output (Lisp_Object, int);
static void handle_child_signal (int);
static void create_pty (Lisp_Object);
@@ -870,7 +871,7 @@
{
pset_status (p, list2 (Qexit, make_number (0)));
p->tick = ++process_tick;
- status_notify (p);
+ status_notify (p, NULL);
redisplay_preserve_echo_area (13);
}
else
@@ -890,7 +891,7 @@
pset_status (p, list2 (Qsignal, make_number (SIGKILL)));
p->tick = ++process_tick;
- status_notify (p);
+ status_notify (p, NULL);
redisplay_preserve_echo_area (13);
}
}
@@ -3997,12 +3998,13 @@
nsecs = 0;
return
- (wait_reading_process_output (secs, nsecs, 0, 0,
+ ((wait_reading_process_output (secs, nsecs, 0, 0,
Qnil,
!NILP (process) ? XPROCESS (process) : NULL,
NILP (just_this_one) ? 0 :
!INTEGERP (just_this_one) ? 1 : -1)
- ? Qt : Qnil);
+ <= 0)
+ ? Qnil : Qt);
}
/* Accept a connection for server process SERVER on CHANNEL. */
@@ -4274,11 +4276,10 @@
(suspending output from other processes). A negative value
means don't run any timers either.
- If WAIT_PROC is specified, then the function returns true if we
- received input from that process before the timeout elapsed.
- Otherwise, return true if we received input from any process. */
+ Return true if we received input from WAIT_PROC, or from any
+ process if WAIT_PROC is null. */
-bool
+int
wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
bool do_display,
Lisp_Object wait_for_cell,
@@ -4293,8 +4294,7 @@
int xerrno;
Lisp_Object proc;
struct timespec timeout, end_time;
- int wait_channel = -1;
- bool got_some_input = 0;
+ int got_some_input = -1;
ptrdiff_t count = SPECPDL_INDEX ();
FD_ZERO (&Available);
@@ -4305,10 +4305,6 @@
&& EQ (XCAR (wait_proc->status), Qexit)))
message1 ("Blocking call to accept-process-output with quit inhibited!!");
- /* If wait_proc is a process to watch, set wait_channel accordingly. */
- if (wait_proc != NULL)
- wait_channel = wait_proc->infd;
-
record_unwind_protect_int (wait_reading_process_output_unwind,
waiting_for_user_input_p);
waiting_for_user_input_p = read_kbd;
@@ -4345,6 +4341,10 @@
if (! NILP (wait_for_cell) && ! NILP (XCAR (wait_for_cell)))
break;
+ /* After reading input, vacuum up any leftovers without waiting. */
+ if (0 <= got_some_input)
+ nsecs = -1;
+
/* Compute time from now till when time limit is up. */
/* Exit if already run out. */
if (nsecs < 0)
@@ -4463,7 +4463,7 @@
/* It's okay for us to do this and then continue with
the loop, since timeout has already been zeroed out. */
clear_waiting_for_input ();
- status_notify (NULL);
+ got_some_input = status_notify (NULL, wait_proc);
if (do_display) redisplay_preserve_echo_area (13);
}
}
@@ -4485,18 +4485,23 @@
while (wait_proc->infd >= 0)
{
int nread = read_process_output (proc, wait_proc->infd);
-
- if (nread == 0)
- break;
-
- if (nread > 0)
- got_some_input = read_some_bytes = 1;
- else if (nread == -1 && (errno == EIO || errno == EAGAIN))
- break;
+ if (nread < 0)
+ {
+ if (errno == EIO || errno == EAGAIN)
+ break;
#ifdef EWOULDBLOCK
- else if (nread == -1 && EWOULDBLOCK == errno)
- break;
+ if (errno == EWOULDBLOCK)
+ break;
#endif
+ }
+ else
+ {
+ if (got_some_input < nread)
+ got_some_input = nread;
+ if (nread == 0)
+ break;
+ read_some_bytes = true;
+ }
}
if (read_some_bytes && do_display)
redisplay_preserve_echo_area (10);
@@ -4527,12 +4532,8 @@
else
Available = input_wait_mask;
Writeok = write_mask;
-#ifdef SELECT_CANT_DO_WRITE_MASK
- check_write = 0;
-#else
- check_write = 1;
-#endif
- check_delay = wait_channel >= 0 ? 0 : process_output_delay_count;
+ check_delay = wait_proc ? 0 : process_output_delay_count;
+ check_write = SELECT_CAN_DO_WRITE_MASK;
}
/* If frame size has changed or the window is newly mapped,
@@ -4558,6 +4559,7 @@
{
nfds = read_kbd ? 0 : 1;
no_avail = 1;
+ FD_ZERO (&Available);
}
if (!no_avail)
@@ -4567,7 +4569,7 @@
/* Set the timeout for adaptive read buffering if any
process has non-zero read_output_skip and non-zero
read_output_delay, and we are not reading output for a
- specific wait_channel. It is not executed if
+ specific process. It is not executed if
Vprocess_adaptive_read_buffering is nil. */
if (process_output_skip && check_delay > 0)
{
@@ -4679,12 +4681,6 @@
report_file_errno ("Failed select", Qnil, xerrno);
}
- if (no_avail)
- {
- FD_ZERO (&Available);
- check_write = 0;
- }
-
/* Check for keyboard input */
/* If there is any, return immediately
to give it higher priority than subprocesses */
@@ -4751,9 +4747,6 @@
handle_input_available_signal (SIGIO);
#endif
- if (! wait_proc)
- got_some_input |= nfds > 0;
-
/* If checking input just got us a size-change event from X,
obey it now if we should. */
if (read_kbd || ! NILP (wait_for_cell))
@@ -4785,12 +4778,6 @@
/* If waiting for this channel, arrange to return as
soon as no more input to be processed. No more
waiting. */
- if (wait_channel == channel)
- {
- wait_channel = -1;
- nsecs = -1;
- got_some_input = 1;
- }
proc = chan_process[channel];
if (NILP (proc))
continue;
@@ -4806,6 +4793,8 @@
buffered-ahead character if we have one. */
nread = read_process_output (proc, channel);
+ if ((!wait_proc || wait_proc == XPROCESS (proc)) && got_some_input < nread)
+ got_some_input = nread;
if (nread > 0)
{
/* Since read_process_output can run a filter,
@@ -5826,7 +5815,7 @@
p->tick = ++process_tick;
if (!nomsg)
{
- status_notify (NULL);
+ status_notify (NULL, NULL);
redisplay_preserve_echo_area (13);
}
}
@@ -6354,14 +6343,20 @@
/* Report all recent events of a change in process status
(either run the sentinel or output a message).
This is usually done while Emacs is waiting for keyboard input
- but can be done at other times. */
-
-static void
-status_notify (struct Lisp_Process *deleting_process)
+ but can be done at other times.
+
+ Return positive if any input was received from WAIT_PROC (or from
+ any process if WAIT_PROC is null), zero if input was attempted but
+ none received, and negative if we didn't even try. */
+
+static int
+status_notify (struct Lisp_Process *deleting_process,
+ struct Lisp_Process *wait_proc)
{
- register Lisp_Object proc;
+ Lisp_Object proc;
Lisp_Object tail, msg;
struct gcpro gcpro1, gcpro2;
+ int got_some_input = -1;
tail = Qnil;
msg = Qnil;
@@ -6391,8 +6386,14 @@
/* Network or serial process not stopped: */
&& ! EQ (p->command, Qt)
&& p->infd >= 0
- && p != deleting_process
- && read_process_output (proc, p->infd) > 0);
+ && p != deleting_process)
+ {
+ int nread = read_process_output (proc, p->infd);
+ if (got_some_input < nread)
+ got_some_input = nread;
+ if (nread <= 0)
+ break;
+ }
/* Get the text to use for the message. */
if (p->raw_status_new)
@@ -6424,6 +6425,7 @@
update_mode_lines = 24; /* In case buffers use %s in mode-line-format. */
UNGCPRO;
+ return got_some_input;
}
DEFUN ("internal-default-process-sentinel", Finternal_default_process_sentinel,
@@ -6635,9 +6637,11 @@
DO_DISPLAY means redisplay should be done to show subprocess
output that arrives.
- Return true if we received input from any process. */
+ Return positive if we received input from WAIT_PROC (or from any
+ process if WAIT_PROC is null), zero if we attempted to receive
+ input but got none, and negative if we didn't even try. */
-bool
+int
wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
bool do_display,
Lisp_Object wait_for_cell,
@@ -6825,7 +6829,7 @@
start_polling ();
- return 0;
+ return -1;
}
#endif /* not subprocesses */
next prev parent reply other threads:[~2014-06-18 21:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <7A63D872-6B1E-4D80-9269-51DC5C95070C@ucsd.edu>
2014-06-18 20:32 ` bug#17805: Fwd: 24.3.50; Crash in -[EmacsApp / Faccept_process_output David Reitter
2014-06-18 21:36 ` Paul Eggert [this message]
2014-06-23 23:09 ` bug#17805: " Chiara Orsini
2014-06-23 23:47 ` Paul Eggert
2014-06-24 2:08 ` David Reitter
2014-06-24 2:42 ` Chiara Orsini
2015-12-26 15:33 ` Lars Ingebrigtsen
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53A2064A.6030903@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=17805@debbugs.gnu.org \
--cc=chiara.orsini@gmail.com \
--cc=david.reitter@gmail.com \
/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 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.