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

[Robert Pluim]

Robert Pluim <address@hidden> writes:

> Matthias Dahl <address@hidden> writes:
>
>> -          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
>>      }
>
> Hi Matthias, I apologize if this has already been mentioned, but did
> you check that this doesn't undo the fix for Bug#21337? The issue
> there as I recall was that FD's were set in Available that didn't
> actually have data to read, hence the need to check the TLS FD's
> separately using tls_available.

Answering my own question: your second patch undoes the fix for
21337. I think it needs to look something like this (I've put some of
the tabs back in to make the diff clearer, adjust according to taste):

diff --git a/src/process.c b/src/process.c
index 11d914aab2..71b638726f 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5392,58 +5392,36 @@ 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++;
-                   }
-               }
+              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, &tls_available);
+                        set++;
+                      }
+                  }
              if (set)
                Available = tls_available;
            }