bug#40665: 28.0.50; tls hang on local ssl

[Derek Zhou]

Robert Pluim writes:

> OK, that does make sense, and might even be more correct, but itʼs a
> bigger change. You'll need more than just me to agree with it.
>
Patch reworked:

 * before the select, check every interesting gnutls stream for
   available data in the buffer
 * if some of them hit, and either there is no wait_proc or the
   wait_proc is one of the gnutls streams with new data, set the select
   timeout to 0
 * after the select, merge the gnutls buffer status into the select
   returns

The patch is not much longer than before, still a net reduction of code
lines. I've done some light testing and haven't found any problem. 

diff --git a/src/process.c b/src/process.c
index 91d426103d..783ce098b3 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5497,6 +5497,10 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
        }
       else
        {
+#ifdef HAVE_GNUTLS
+         int tls_nfds;
+         fd_set tls_available;
+#endif
          /* 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
@@ -5566,6 +5570,36 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
            }
 #endif
 
+#ifdef HAVE_GNUTLS
+          /* GnuTLS buffers data internally. We need to check if some
+            data is available in the buffers manually before the select.
+            And if so, we need to skip the select which could block */
+         FD_ZERO (&tls_available);
+         tls_nfds = 0;
+         for (channel = 0; channel < FD_SETSIZE; ++channel)
+           if (! NILP (chan_process[channel]) && FD_ISSET(channel, &Available))
+             {
+               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))
+                 {
+                   tls_nfds++;
+                   eassert (p->infd == channel);
+                   FD_SET (p->infd, &tls_available);
+                 }
+             }
+         /* if wait_proc is somebody else, we have to wait in select as usual.
+            Otherwisr, clobber the timeout */
+         if ((tls_nfds > 0) &&
+             (!wait_proc ||
+              (wait_proc->infd >= 0 &&
+               FD_ISSET(wait_proc->infd, &tls_available))))
+           timeout = make_timespec (0, 0);
+#endif
+
 /* Non-macOS HAVE_GLIB builds call thread_select in xgselect.c.  */
 #if defined HAVE_GLIB && !defined HAVE_NS
          nfds = xg_select (max_desc + 1,
@@ -5584,60 +5618,22 @@ 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)
+      /* merge tls_available into Available */
+      if (tls_nfds > 0)
+       {
+         if (nfds == 0 || (nfds < 0 && errno == EINTR))
            {
-             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.  */
-                 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;
+             /* fast path, just copy */
+             nfds = tls_nfds;
+             Available = tls_available;
            }
+         else if (nfds > 0)
+           /* slow path, merge one by one.
+              Note: nfds does not need to be accurate, just positive is enough 
*/
+           for (channel = 0; channel < FD_SETSIZE; ++channel)
+             if (FD_ISSET(channel, &tls_available))
+                 FD_SET(channel, &Available);
+       }
 #endif
        }