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

From: Robert Pluim <[email-masked]>
Date: Wed, 22 Apr 2020 18:14:16 +0200

>>>>> On Wed, 22 Apr 2020 15:16:12 +0000 (UTC), Derek Zhou <[email-masked]> said:
    >> 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> My counter argument is if we really only care about some of the the fds
    Derek> but not all the fds, the proper way is to let select know by passing in
    Derek> the proper narrower set of fds, maybe the code is already this way? It is very
    Derek> complicated so I am not sure. I am checking only those fds that are both
    Derek> 1, gnutls managed, and the 2 set in the input for readfds for the
    Derek> select, so I believe it is the right thing.

Thatʼs not quite it. 'wait_reading_process_output' broadly does one of:

    1. Read from some fds, give me what you have or timeout
    2. Read from some fds, wait until you have something for this
    specific process, or timeout
    3. Read *only* for this process, wait until you have something, or
    timeout

[1] is very common, [3] is not. The case Iʼm discussing is [2] (via
accept-process-output), and as it stands you've transformed it into a
form of [1] (and yes, in [3] the select is only done for the fd for a
specific process).

    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.

    Derek> I am proposing doing a zero timeout select ONLY if the gnutls buffer
    Derek> check already flags some of the channels. This way we can also select those
    Derek> FDs that are not gnutls managed, but already ready to read at the same
    Derek> moment. It is closer to the origin intention of the select, I
    Derek> believe. If the gnutls buffer check does not flag anything of cause we do
    Derek> the select with timeout exactly as before. My current patch may leave
    Derek> out some ready fd unchecked until the next round.

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.

Robert