bug#40665: 28.0.50; tls hang on local ssl

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

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


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.


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

diff --git a/src/process.c b/src/process.c
index 91d426103d..afbe861533 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5566,7 +5566,32 @@ 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 */
+	  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, &Available);
+		  }
+	      }
+	  /* don't select if we have something here */
+	  if (nfds > 0)
+	    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 +5607,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.  */