Subject: Re: bug#40665: 28.0.50; tls hang on local ssl

From: Robert Pluim <[email-masked]>
Date: Wed, 22 Apr 2020 15:12:40 +0200

>>>>> On Wed, 22 Apr 2020 12:25:59 +0000 (UTC), Derek Zhou <[email-masked]> said:

    Derek> Robert Pluim writes:

    >>>>>>> On Tue, 21 Apr 2020 22:29:30 +0000 (UTC), Derek Zhou <[email-masked]> said:
    >> 
    Derek> Version 3, add safty guard for tls read detection. Please use this one.
    Derek> You may want to review this carefully.
    >> 
    >> I haven't tested it yet, but what about wait_proc?
    >>

Iʼve tested it lightly and it fixes <https://www.opensource.com>

    Derek> I don't quite understand the wait_proc business. The idea of the patch
    Derek> is to detect that out of all the fds that are going to be selected, how
    Derek> many are gnutls managed and are ready from the gnutls buffer? If the
    Derek> answer is positive, we skip the select and pretend the select return
    Derek> those fds only. I think this is safe; because it is one of the possible
    Derek> and legal return of the select, wait_proc or not.

The reason for checking wait_proc is to allow 'accept-process-output'
to specify that emacs should return only when there is data for that
specific process, with your patch it can return if there is any data
in the TLS buffers for any connection, but none for wait_proc. That
would make 'accept-process-output' return earlier than expected, or
even return for the case where the timeout is infinite.

A quick survey of the emacs sources shows almost every call to
'accept-process-output' passes in wait_proc, so I think that your
change as it stands is too risky. With a check for wait_proc it might
be OK.

    Derek> Another way is to still do a zero timeout select, and merge the gnutls
    Derek> ready set with the select ready set. It is more intrusive but probably
    Derek> closer to the original intent of the code. I can write the path that way
    Derek> if you want.

I donʼt think we always do a zero timeout select. This sounds even
riskier.

Robert