[Emacs-diffs] trunk r117328: Avoid hangs in accept-process-output.

[Paul Eggert]

------------------------------------------------------------
revno: 117328
revision-id: address@hidden
parent: address@hidden
fixes bug: http://debbugs.gnu.org/17647
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Fri 2014-06-13 08:55:48 -0700
message:
  Avoid hangs in accept-process-output.
  
  * 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.
modified:
  src/ChangeLog                  changelog-20091113204419-o5vbwnq5f7feedwu-1438
  src/lisp.h                     lisp.h-20091113204419-o5vbwnq5f7feedwu-253
  src/process.c                  process.c-20091113204419-o5vbwnq5f7feedwu-462

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2014-06-13 14:42:43 +0000
+++ b/src/ChangeLog     2014-06-13 15:55:48 +0000
@@ -1,3 +1,15 @@
+2014-06-13  Paul Eggert  <address@hidden>
+
+       Avoid hangs in accept-process-output (Bug#17647).
+       * 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-13  Eli Zaretskii  <address@hidden>
 
        * menu.c (Fx_popup_menu): Don't call the frame's menu_show_hook if

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2014-06-08 18:27:22 +0000
+++ b/src/lisp.h        2014-06-13 15:55:48 +0000
@@ -4178,10 +4178,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'
--- a/src/process.c     2014-06-09 20:31:06 +0000
+++ b/src/process.c     2014-06-13 15:55:48 +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);
@@ -860,7 +861,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
@@ -880,7 +881,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);
        }
     }
@@ -3986,12 +3987,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.  */
@@ -4262,10 +4264,11 @@
      (suspending output from other processes).  A negative value
      means don't run any timers either.
 
-   Return true if we received input from WAIT_PROC, or from any
-   process if WAIT_PROC is null.  */
+   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,
@@ -4280,8 +4283,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);
@@ -4292,10 +4294,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;
@@ -4332,6 +4330,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)
@@ -4450,7 +4452,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);
            }
        }
@@ -4472,18 +4474,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);
@@ -4514,12 +4521,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,
@@ -4545,6 +4548,7 @@
        {
          nfds = read_kbd ? 0 : 1;
          no_avail = 1;
+         FD_ZERO (&Available);
        }
 
       if (!no_avail)
@@ -4554,7 +4558,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)
            {
@@ -4667,12 +4671,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 */
@@ -4739,9 +4737,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))
@@ -4773,12 +4768,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;
@@ -4794,6 +4783,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,
@@ -5814,7 +5805,7 @@
       p->tick = ++process_tick;
       if (!nomsg)
        {
-         status_notify (NULL);
+         status_notify (NULL, NULL);
          redisplay_preserve_echo_area (13);
        }
     }
@@ -6337,14 +6328,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;
@@ -6374,8 +6371,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)
@@ -6407,6 +6410,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,
@@ -6618,9 +6622,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,
@@ -6808,7 +6814,7 @@
 
   start_polling ();
 
-  return 0;
+  return -1;
 }
 
 #endif /* not subprocesses */