bug#40665: 28.0.50; tls hang on local ssl

[Derek Zhou]

Robert,

My patch still apply cleanly to either emacs-27 or master branch. I don't know 
who
else can review this, so I am adding a few random people from the git
log, sorry for the spam. Just don't it to sit in the dust and got forgotten. 

Derek

Derek Zhou writes:

> 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
>       }
>