bug#40665: 28.0.50; tls hang on local ssl

[Derek Zhou]

Derek Zhou writes:

> Derek Zhou writes:
>
>> Robert Pluim writes:
>>
>>> Thatʼs always possible. You'd have to stick some instrumentation in
>>> things like connect_network_socket and wait_reading_process_output to
>>> narrow it down.
>>>
>> I think what happened is gnutls's internal buffering exhausts the
>> available data from the socket, so select blocks. There is an comment in
>> the code said gnutls leave one byte in the socket for select, but I
>> don't think it is doing this anymore. The following patch move the check
>> before the select and skip the select if there is data to be read in
>> gnutls. It fixed the occational stall in https.
>
> Sorry, the previous patch was wrong. Cannot reuse the fd_set intended
> for select. Corrected:
>
Version 3, add safty guard for tls read detection. Please use this one.
You may want to review this carefully.

diff --git a/src/process.c b/src/process.c
index 91d426103d..683c28b3fd 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5566,7 +5566,38 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
            }
 #endif
 
-/* Non-macOS HAVE_GLIB builds call thread_select in xgselect.c.  */
+#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_set tls_available;
+           FD_ZERO (&tls_available);
+           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))
+                   {
+                     nfds++;
+                     eassert (p->infd == channel);
+                     FD_SET (p->infd, &tls_available);
+                   }
+               }
+         /* don't select if we have something here */
+           if (nfds > 0) {
+             Available = tls_available;
+             goto SELECT_END;
+           }
+         }
+#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,
                            &Available, (check_write ? &Writeok : 0),
@@ -5582,65 +5613,9 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
                                (check_write ? &Writeok : 0),
                                NULL, &timeout, NULL);
 #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)
-           {
-             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;
-           }
-#endif
        }
 
+    SELECT_END:
       xerrno = errno;
 
       /* Make C-g and alarm signals set flags again.  */