bug#40665: 28.0.50; tls hang on local ssl

[Derek Zhou <derek@3qin.us>]

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


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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: check_pending_before_select.patch --]
[-- Type: text/x-diff, Size: 3376 bytes --]

diff --git a/src/process.c b/src/process.c
index 91d426103d..49b034b1a7 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]))
+		{
+		  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.  */