Re: wait_reading_process_ouput hangs in certain cases (w/ patches)

[Matthias Dahl <ml_emacs-lists@binary-island.eu>]

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

Hello Lars...

On 13/03/18 17:32, Lars Ingebrigtsen wrote:

> Can you resend as a single patch set?  I'm not 100% sure what patches to
> apply...

Sure. Attached you will find a combined patch for the master as well as
the current emacs-26 branch. It contains all relevant fixes up until
now.

FYI, the relevant patches (also attached for completeness sake):

Only Emacs-26 branch:

0001-Add-process-output-read-accounting.patch
0002-Fix-wait_reading_process_output-wait_proc-hang.patch

This got applied to the master branch as one:
4ba32858d61eee16f17b51aca01c15211a0912f8

Emacs master and Emacs-26 branch:

0001-Fix-GnuTLS-error-handling.patch
0002-Always-check-GnuTLS-sessions-for-available-data.patch
0003-Make-xg_select-behave-more-like-pselect.patch

Hope that helps. Thanks for testing. Looking forward to your results! :)

Have a nice day,
Matthias

-- 
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-process-output-read-accounting.patch --]
[-- Type: text/x-patch; name="0001-Add-process-output-read-accounting.patch", Size: 1597 bytes --]

From 94e0dc26f45e1a06881b016dd26446c43d339a4d Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
Date: Tue, 24 Oct 2017 15:55:53 +0200
Subject: [PATCH 1/2] Add process output read accounting

This tracks the bytes read from a process' stdin which is not used
anywhere yet but required for follow-up work.
* src/process.c (read_process_output): Track bytes read from a process.
* src/process.h (struct Lisp_Process): Add nbytes_read to track bytes
read from a process.
---
 src/process.c | 3 +++
 src/process.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/src/process.c b/src/process.c
index 2ec10b12ec..17fdf592ec 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5889,6 +5889,9 @@ read_process_output (Lisp_Object proc, int channel)
 	return nbytes;
       coding->mode |= CODING_MODE_LAST_BLOCK;
     }
+  
+  /* Ignore carryover, it's been added by a previous iteration already.  */
+  p->nbytes_read += nbytes;
 
   /* Now set NBYTES how many bytes we must decode.  */
   nbytes += carryover;
diff --git a/src/process.h b/src/process.h
index ab468b18c5..6464a8cc61 100644
--- a/src/process.h
+++ b/src/process.h
@@ -129,6 +129,8 @@ struct Lisp_Process
     pid_t pid;
     /* Descriptor by which we read from this process.  */
     int infd;
+    /* Byte-count modulo (UINTMAX_MAX + 1) for process output read from `infd'.  */
+    uintmax_t nbytes_read;
     /* Descriptor by which we write to this process.  */
     int outfd;
     /* Descriptors that were created for this process and that need
-- 
2.16.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-GnuTLS-error-handling.patch --]
[-- Type: text/x-patch; name="0001-Fix-GnuTLS-error-handling.patch", Size: 2146 bytes --]

From 841d0fbe37642bc14c4bcd515cfbc91a25ba179b Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
Date: Mon, 12 Mar 2018 15:33:45 +0100
Subject: [PATCH 1/3] Fix GnuTLS error handling

* src/gnutls.c (emacs_gnutls_read): All error handling should be done
in `emacs_gnutls_handle_error', move handling of
GNUTLS_E_UNEXPECTED_PACKET_LENGTH accordingly.
We always need to set `errno' in case of an error, since later error
handling (e.g. `wait_reading_process_output') depends on it and GnuTLS
does not set errno by itself. We'll otherwise have random errno values
which can cause erratic behavior and hangs.
(emacs_gnutls_handle_error): GNUTLS_E_UNEXPECTED_PACKET_LENGTH is only
returned for premature connection termination on GnuTLS < 3.0 and is
otherwise a real error and should not be gobbled up.
---
 src/gnutls.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 903393fed1..e7d0d3d845 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char *buf, ptrdiff_t nbyte)
   rtnval = gnutls_record_recv (state, buf, nbyte);
   if (rtnval >= 0)
     return rtnval;
-  else if (rtnval == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
-    /* The peer closed the connection. */
-    return 0;
   else if (emacs_gnutls_handle_error (state, rtnval))
-    /* non-fatal error */
-    return -1;
-  else {
-    /* a fatal error occurred */
-    return 0;
-  }
+    {
+      /* non-fatal error */
+      errno = EAGAIN;
+      return -1;
+    }
+  else
+    {
+      /* a fatal error occurred */
+      errno = EPROTO;
+      return 0;
+    }
 }
 
 static char const *
@@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
 	 connection.  */
 # ifdef HAVE_GNUTLS3
       if (err == GNUTLS_E_PREMATURE_TERMINATION)
-	level = 3;
+# else
+      if (err == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
 # endif
+	level = 3;
 
       GNUTLS_LOG2 (level, max_log_level, "fatal error:", str);
       ret = false;
-- 
2.16.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Always-check-GnuTLS-sessions-for-available-data.patch --]
[-- Type: text/x-patch; name="0002-Always-check-GnuTLS-sessions-for-available-data.patch", Size: 4596 bytes --]

From f5b979da67595da4d91f7b7a4eb979deae6a7321 Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
Date: Mon, 12 Mar 2018 16:07:55 +0100
Subject: [PATCH 2/3] Always check GnuTLS sessions for available data

* src/process.c (wait_reading_process_output): GnuTLS buffers data
internally and as such there is no guarantee that a select() call on
the underlying kernel socket will report available data if all data
has already been buffered. Prior to GnuTLS < 2.12, lowat mode was the
default which left bytes back in the kernel socket, so a select() call
would work. With GnuTLS >= 2.12 (the now required version for Emacs),
that default changed to non-lowat mode (and we don't set it otherwise)
and was subsequently completely removed with GnuTLS >= 3.0.
So, to properly handle GnuTLS sessions, we need to iterate through all
channels, check for available data manually and set the concerning fds
accordingly. Otherwise we might stall/delay unnecessarily or worse.
This also applies to the !just_wait_proc && wait_proc case, which was
previously handled improperly (only wait_proc was checked) and could
cause problems if sessions did have any dependency on one another
through e.g. higher up program logic and waited for one another.
---
 src/process.c | 77 +++++++++++++++++++----------------------------------------
 1 file changed, 24 insertions(+), 53 deletions(-)

diff --git a/src/process.c b/src/process.c
index 9b9b9f3550..f8fed56d5b 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5392,60 +5392,31 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif	/* !HAVE_GLIB */
 
 #ifdef HAVE_GNUTLS
-          /* GnuTLS buffers data internally.  In lowat mode it leaves
-             some data in the TCP buffers so that select works, but
-             with custom pull/push functions we need to check if some
-             data is available in the buffers manually.  */
-          if (nfds == 0)
+          /* GnuTLS buffers data internally. select() will only report
+             available data for the underlying kernel sockets API, not
+             what has been buffered internally. As such, we need to loop
+             through all channels and check for available data manually.  */
+          if (nfds >= 0)
 	    {
-	      fd_set tls_available;
-	      int set = 0;
-
-	      FD_ZERO (&tls_available);
-	      if (! wait_proc)
-		{
-		  /* We're not waiting on a specific process, so loop
-		     through all the channels and check for data.
-		     This is a workaround needed for some versions of
-		     the gnutls library -- 2.12.14 has been confirmed
-		     to need it.  See
-		     http://comments.gmane.org/gmane.emacs.devel/145074 */
-		  for (channel = 0; channel < FD_SETSIZE; ++channel)
-		    if (! NILP (chan_process[channel]))
-		      {
-			struct Lisp_Process *p =
-			  XPROCESS (chan_process[channel]);
-			if (p && p->gnutls_p && p->gnutls_state
-			    && ((emacs_gnutls_record_check_pending
-				 (p->gnutls_state))
-				> 0))
-			  {
-			    nfds++;
-			    eassert (p->infd == channel);
-			    FD_SET (p->infd, &tls_available);
-			    set++;
-			  }
-		      }
-		}
-	      else
-		{
-		  /* Check this specific channel.  */
-		  if (wait_proc->gnutls_p /* Check for valid process.  */
-		      && wait_proc->gnutls_state
-		      /* Do we have pending data?  */
-		      && ((emacs_gnutls_record_check_pending
-			   (wait_proc->gnutls_state))
-			  > 0))
-		    {
-		      nfds = 1;
-		      eassert (0 <= wait_proc->infd);
-		      /* Set to Available.  */
-		      FD_SET (wait_proc->infd, &tls_available);
-		      set++;
-		    }
-		}
-	      if (set)
-		Available = tls_available;
+              for (channel = 0; channel < FD_SETSIZE; ++channel)
+                if (! NILP (chan_process[channel]))
+                  {
+                    struct Lisp_Process *p =
+                      XPROCESS (chan_process[channel]);
+
+                    if (just_wait_proc && p != wait_proc)
+                      continue;
+
+                    if (p && p->gnutls_p && p->gnutls_state
+                        && ((emacs_gnutls_record_check_pending
+                             (p->gnutls_state))
+                            > 0))
+                      {
+                        nfds++;
+                        eassert (p->infd == channel);
+                        FD_SET (p->infd, &Available);
+                      }
+                  }
 	    }
 #endif
 	}
-- 
2.16.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0002-Fix-wait_reading_process_output-wait_proc-hang.patch --]
[-- Type: text/x-patch; name="0002-Fix-wait_reading_process_output-wait_proc-hang.patch", Size: 3307 bytes --]

From b9c05bbfb4559b21deb0ea4e156430dedb60ce41 Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
Date: Tue, 6 Feb 2018 15:24:15 +0100
Subject: [PATCH 2/2] Fix wait_reading_process_output wait_proc hang

* src/process.c (wait_reading_process_output): If called recursively
through timers and/or process filters via accept-process-output, it is
possible that the output of wait_proc has already been read by one of
those recursive calls, leaving the original call hanging forever if no
further output arrives through that fd and no timeout has been set.
Fix that by using the process read accounting to keep track of how
many bytes have been read and use that as a condition to break out
of the infinite loop and return to the caller as well as to calculate
the proper return value (if a wait_proc is given that is).
---
 src/process.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/process.c b/src/process.c
index 17fdf592ec..0abbd5fa8e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5006,6 +5006,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   struct timespec got_output_end_time = invalid_timespec ();
   enum { MINIMUM = -1, TIMEOUT, INFINITY } wait;
   int got_some_output = -1;
+  uintmax_t prev_wait_proc_nbytes_read = wait_proc ? wait_proc->nbytes_read : 0;
 #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS
   bool retry_for_async;
 #endif
@@ -5460,6 +5461,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       if (nfds == 0)
 	{
           /* Exit the main loop if we've passed the requested timeout,
+             or have read some bytes from our wait_proc (either directly
+             in this call or indirectly through timers / process filters),
              or aren't skipping processes and got some output and
              haven't lowered our timeout due to timers or SIGIO and
              have waited a long amount of time due to repeated
@@ -5467,7 +5470,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	  struct timespec huge_timespec
 	    = make_timespec (TYPE_MAXIMUM (time_t), 2 * TIMESPEC_RESOLUTION);
 	  struct timespec cmp_time = huge_timespec;
-	  if (wait < TIMEOUT)
+	  if (wait < TIMEOUT
+              || (wait_proc
+                  && wait_proc->nbytes_read != prev_wait_proc_nbytes_read))
 	    break;
 	  if (wait == TIMEOUT)
 	    cmp_time = end_time;
@@ -5772,6 +5777,15 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       maybe_quit ();
     }
 
+  /* Timers and/or process filters that we have run could have themselves called
+     `accept-process-output' (and by that indirectly this function), thus
+     possibly reading some (or all) output of wait_proc without us noticing it.
+     This could potentially lead to an endless wait (dealt with earlier in the
+     function) and/or a wrong return value (dealt with here).  */
+  if (wait_proc && wait_proc->nbytes_read != prev_wait_proc_nbytes_read)
+    got_some_output = min (INT_MAX, (wait_proc->nbytes_read
+                                     - prev_wait_proc_nbytes_read));
+
   return got_some_output;
 }
 \f
-- 
2.16.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0003-Make-xg_select-behave-more-like-pselect.patch --]
[-- Type: text/x-patch; name="0003-Make-xg_select-behave-more-like-pselect.patch", Size: 1322 bytes --]

From 2f44ef364bc320dfd73febc51f0c0da862db49b1 Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
Date: Tue, 13 Mar 2018 15:35:16 +0100
Subject: [PATCH 3/3] Make xg_select() behave more like pselect()

* src/xgselect.c (xg_select): If no file descriptors have data ready,
pselect() clears the passed in fd sets whereas xg_select() does not
which caused Bug#21337 for `wait_reading_process_output'.
Clear the passed in sets if no fds are ready but leave them untouched
if pselect() returns an error -- just like pselect() does itself.
---
 src/xgselect.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/xgselect.c b/src/xgselect.c
index fedd3127ef..f68982143e 100644
--- a/src/xgselect.c
+++ b/src/xgselect.c
@@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds,
             ++retval;
         }
     }
+  else if (nfds == 0)
+    {
+      // pselect() clears the file descriptor sets if no fd is ready (but
+      // not if an error occurred), so should we to be compatible. (Bug#21337)
+      if (rfds) FD_ZERO (rfds);
+      if (wfds) FD_ZERO (wfds);
+      if (efds) FD_ZERO (efds);
+    }
 
   /* If Gtk+ is in use eventually gtk_main_iteration will be called,
      unless retval is zero.  */
-- 
2.16.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: emacs-emacs26-combined.patch --]
[-- Type: text/x-patch; name="emacs-emacs26-combined.patch", Size: 8083 bytes --]

diff --git a/src/gnutls.c b/src/gnutls.c
index 903393fed1..e7d0d3d845 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char *buf, ptrdiff_t nbyte)
   rtnval = gnutls_record_recv (state, buf, nbyte);
   if (rtnval >= 0)
     return rtnval;
-  else if (rtnval == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
-    /* The peer closed the connection. */
-    return 0;
   else if (emacs_gnutls_handle_error (state, rtnval))
-    /* non-fatal error */
-    return -1;
-  else {
-    /* a fatal error occurred */
-    return 0;
-  }
+    {
+      /* non-fatal error */
+      errno = EAGAIN;
+      return -1;
+    }
+  else
+    {
+      /* a fatal error occurred */
+      errno = EPROTO;
+      return 0;
+    }
 }
 
 static char const *
@@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
 	 connection.  */
 # ifdef HAVE_GNUTLS3
       if (err == GNUTLS_E_PREMATURE_TERMINATION)
-	level = 3;
+# else
+      if (err == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
 # endif
+	level = 3;
 
       GNUTLS_LOG2 (level, max_log_level, "fatal error:", str);
       ret = false;
diff --git a/src/process.c b/src/process.c
index b201e9b6ac..c1116c1deb 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5005,6 +5005,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
   struct timespec got_output_end_time = invalid_timespec ();
   enum { MINIMUM = -1, TIMEOUT, INFINITY } wait;
   int got_some_output = -1;
+  uintmax_t prev_wait_proc_nbytes_read = wait_proc ? wait_proc->nbytes_read : 0;
 #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS
   bool retry_for_async;
 #endif
@@ -5390,60 +5391,31 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif	/* !HAVE_GLIB */
 
 #ifdef HAVE_GNUTLS
-          /* GnuTLS buffers data internally.  In lowat mode it leaves
-             some data in the TCP buffers so that select works, but
-             with custom pull/push functions we need to check if some
-             data is available in the buffers manually.  */
-          if (nfds == 0)
+          /* GnuTLS buffers data internally. select() will only report
+             available data for the underlying kernel sockets API, not
+             what has been buffered internally. As such, we need to loop
+             through all channels and check for available data manually.  */
+          if (nfds >= 0)
 	    {
-	      fd_set tls_available;
-	      int set = 0;
-
-	      FD_ZERO (&tls_available);
-	      if (! wait_proc)
-		{
-		  /* We're not waiting on a specific process, so loop
-		     through all the channels and check for data.
-		     This is a workaround needed for some versions of
-		     the gnutls library -- 2.12.14 has been confirmed
-		     to need it.  See
-		     http://comments.gmane.org/gmane.emacs.devel/145074 */
-		  for (channel = 0; channel < FD_SETSIZE; ++channel)
-		    if (! NILP (chan_process[channel]))
-		      {
-			struct Lisp_Process *p =
-			  XPROCESS (chan_process[channel]);
-			if (p && p->gnutls_p && p->gnutls_state
-			    && ((emacs_gnutls_record_check_pending
-				 (p->gnutls_state))
-				> 0))
-			  {
-			    nfds++;
-			    eassert (p->infd == channel);
-			    FD_SET (p->infd, &tls_available);
-			    set++;
-			  }
-		      }
-		}
-	      else
-		{
-		  /* Check this specific channel.  */
-		  if (wait_proc->gnutls_p /* Check for valid process.  */
-		      && wait_proc->gnutls_state
-		      /* Do we have pending data?  */
-		      && ((emacs_gnutls_record_check_pending
-			   (wait_proc->gnutls_state))
-			  > 0))
-		    {
-		      nfds = 1;
-		      eassert (0 <= wait_proc->infd);
-		      /* Set to Available.  */
-		      FD_SET (wait_proc->infd, &tls_available);
-		      set++;
-		    }
-		}
-	      if (set)
-		Available = tls_available;
+              for (channel = 0; channel < FD_SETSIZE; ++channel)
+                if (! NILP (chan_process[channel]))
+                  {
+                    struct Lisp_Process *p =
+                      XPROCESS (chan_process[channel]);
+
+                    if (just_wait_proc && p != wait_proc)
+                      continue;
+
+                    if (p && p->gnutls_p && p->gnutls_state
+                        && ((emacs_gnutls_record_check_pending
+                             (p->gnutls_state))
+                            > 0))
+                      {
+                        nfds++;
+                        eassert (p->infd == channel);
+                        FD_SET (p->infd, &Available);
+                      }
+                  }
 	    }
 #endif
 	}
@@ -5459,6 +5431,8 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       if (nfds == 0)
 	{
           /* Exit the main loop if we've passed the requested timeout,
+             or have read some bytes from our wait_proc (either directly
+             in this call or indirectly through timers / process filters),
              or aren't skipping processes and got some output and
              haven't lowered our timeout due to timers or SIGIO and
              have waited a long amount of time due to repeated
@@ -5466,7 +5440,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 	  struct timespec huge_timespec
 	    = make_timespec (TYPE_MAXIMUM (time_t), 2 * TIMESPEC_RESOLUTION);
 	  struct timespec cmp_time = huge_timespec;
-	  if (wait < TIMEOUT)
+	  if (wait < TIMEOUT
+              || (wait_proc
+                  && wait_proc->nbytes_read != prev_wait_proc_nbytes_read))
 	    break;
 	  if (wait == TIMEOUT)
 	    cmp_time = end_time;
@@ -5781,6 +5757,15 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
       maybe_quit ();
     }
 
+  /* Timers and/or process filters that we have run could have themselves called
+     `accept-process-output' (and by that indirectly this function), thus
+     possibly reading some (or all) output of wait_proc without us noticing it.
+     This could potentially lead to an endless wait (dealt with earlier in the
+     function) and/or a wrong return value (dealt with here).  */
+  if (wait_proc && wait_proc->nbytes_read != prev_wait_proc_nbytes_read)
+    got_some_output = min (INT_MAX, (wait_proc->nbytes_read
+                                     - prev_wait_proc_nbytes_read));
+
   return got_some_output;
 }
 \f
@@ -5899,6 +5884,9 @@ read_process_output (Lisp_Object proc, int channel)
       coding->mode |= CODING_MODE_LAST_BLOCK;
     }
 
+  /* Ignore carryover, it's been added by a previous iteration already.  */
+  p->nbytes_read += nbytes;
+
   /* Now set NBYTES how many bytes we must decode.  */
   nbytes += carryover;
 
diff --git a/src/process.h b/src/process.h
index ab468b18c5..6464a8cc61 100644
--- a/src/process.h
+++ b/src/process.h
@@ -129,6 +129,8 @@ struct Lisp_Process
     pid_t pid;
     /* Descriptor by which we read from this process.  */
     int infd;
+    /* Byte-count modulo (UINTMAX_MAX + 1) for process output read from `infd'.  */
+    uintmax_t nbytes_read;
     /* Descriptor by which we write to this process.  */
     int outfd;
     /* Descriptors that were created for this process and that need
diff --git a/src/xgselect.c b/src/xgselect.c
index fedd3127ef..f68982143e 100644
--- a/src/xgselect.c
+++ b/src/xgselect.c
@@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds,
             ++retval;
         }
     }
+  else if (nfds == 0)
+    {
+      // pselect() clears the file descriptor sets if no fd is ready (but
+      // not if an error occurred), so should we to be compatible. (Bug#21337)
+      if (rfds) FD_ZERO (rfds);
+      if (wfds) FD_ZERO (wfds);
+      if (efds) FD_ZERO (efds);
+    }
 
   /* If Gtk+ is in use eventually gtk_main_iteration will be called,
      unless retval is zero.  */

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: emacs-master-combined.patch --]
[-- Type: text/x-patch; name="emacs-master-combined.patch", Size: 5017 bytes --]

diff --git a/src/gnutls.c b/src/gnutls.c
index 903393fed1..e7d0d3d845 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char *buf, ptrdiff_t nbyte)
   rtnval = gnutls_record_recv (state, buf, nbyte);
   if (rtnval >= 0)
     return rtnval;
-  else if (rtnval == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
-    /* The peer closed the connection. */
-    return 0;
   else if (emacs_gnutls_handle_error (state, rtnval))
-    /* non-fatal error */
-    return -1;
-  else {
-    /* a fatal error occurred */
-    return 0;
-  }
+    {
+      /* non-fatal error */
+      errno = EAGAIN;
+      return -1;
+    }
+  else
+    {
+      /* a fatal error occurred */
+      errno = EPROTO;
+      return 0;
+    }
 }
 
 static char const *
@@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
 	 connection.  */
 # ifdef HAVE_GNUTLS3
       if (err == GNUTLS_E_PREMATURE_TERMINATION)
-	level = 3;
+# else
+      if (err == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
 # endif
+	level = 3;
 
       GNUTLS_LOG2 (level, max_log_level, "fatal error:", str);
       ret = false;
diff --git a/src/process.c b/src/process.c
index 9b9b9f3550..f8fed56d5b 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5392,60 +5392,31 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif	/* !HAVE_GLIB */
 
 #ifdef HAVE_GNUTLS
-          /* GnuTLS buffers data internally.  In lowat mode it leaves
-             some data in the TCP buffers so that select works, but
-             with custom pull/push functions we need to check if some
-             data is available in the buffers manually.  */
-          if (nfds == 0)
+          /* GnuTLS buffers data internally. select() will only report
+             available data for the underlying kernel sockets API, not
+             what has been buffered internally. As such, we need to loop
+             through all channels and check for available data manually.  */
+          if (nfds >= 0)
 	    {
-	      fd_set tls_available;
-	      int set = 0;
-
-	      FD_ZERO (&tls_available);
-	      if (! wait_proc)
-		{
-		  /* We're not waiting on a specific process, so loop
-		     through all the channels and check for data.
-		     This is a workaround needed for some versions of
-		     the gnutls library -- 2.12.14 has been confirmed
-		     to need it.  See
-		     http://comments.gmane.org/gmane.emacs.devel/145074 */
-		  for (channel = 0; channel < FD_SETSIZE; ++channel)
-		    if (! NILP (chan_process[channel]))
-		      {
-			struct Lisp_Process *p =
-			  XPROCESS (chan_process[channel]);
-			if (p && p->gnutls_p && p->gnutls_state
-			    && ((emacs_gnutls_record_check_pending
-				 (p->gnutls_state))
-				> 0))
-			  {
-			    nfds++;
-			    eassert (p->infd == channel);
-			    FD_SET (p->infd, &tls_available);
-			    set++;
-			  }
-		      }
-		}
-	      else
-		{
-		  /* Check this specific channel.  */
-		  if (wait_proc->gnutls_p /* Check for valid process.  */
-		      && wait_proc->gnutls_state
-		      /* Do we have pending data?  */
-		      && ((emacs_gnutls_record_check_pending
-			   (wait_proc->gnutls_state))
-			  > 0))
-		    {
-		      nfds = 1;
-		      eassert (0 <= wait_proc->infd);
-		      /* Set to Available.  */
-		      FD_SET (wait_proc->infd, &tls_available);
-		      set++;
-		    }
-		}
-	      if (set)
-		Available = tls_available;
+              for (channel = 0; channel < FD_SETSIZE; ++channel)
+                if (! NILP (chan_process[channel]))
+                  {
+                    struct Lisp_Process *p =
+                      XPROCESS (chan_process[channel]);
+
+                    if (just_wait_proc && p != wait_proc)
+                      continue;
+
+                    if (p && p->gnutls_p && p->gnutls_state
+                        && ((emacs_gnutls_record_check_pending
+                             (p->gnutls_state))
+                            > 0))
+                      {
+                        nfds++;
+                        eassert (p->infd == channel);
+                        FD_SET (p->infd, &Available);
+                      }
+                  }
 	    }
 #endif
 	}
diff --git a/src/xgselect.c b/src/xgselect.c
index fedd3127ef..f68982143e 100644
--- a/src/xgselect.c
+++ b/src/xgselect.c
@@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds,
             ++retval;
         }
     }
+  else if (nfds == 0)
+    {
+      // pselect() clears the file descriptor sets if no fd is ready (but
+      // not if an error occurred), so should we to be compatible. (Bug#21337)
+      if (rfds) FD_ZERO (rfds);
+      if (wfds) FD_ZERO (wfds);
+      if (efds) FD_ZERO (efds);
+    }
 
   /* If Gtk+ is in use eventually gtk_main_iteration will be called,
      unless retval is zero.  */