bug#65211: 30.0.50; threads and accept-process-output

[Robert Pluim]

>>>>> On Thu, 10 Aug 2023 17:04:48 +0200, Helmut Eller <eller.helmut@gmail.com> 
>>>>> said:

    Helmut> The test case from below hangs when executed with
    Helmut>   emacs -Q --batch -l apo-tests.el -f ert-run-tests-batch-and-exit

    Helmut> It's not 100% reproducible, but it seems to hang 80% of the time.

    Helmut> The test is fairly complicated and involves a TLS connection, a
    Helmut> sub-process and a background thread.  The background thread starts a
    Helmut> sub-process and reads all its output until the process terminates.  
Then
    Helmut> the main thread opens a TLS connection, sends a HTTP request and 
tries
    Helmut> to read the response.  At this point, accept-process-output hangs 
even
    Helmut> though there is output available.

    Helmut> I think the reason for this problem is that the sub-process uses a 
file
    Helmut> descriptor (usually number 6) and sets
    Helmut> fd_callback_info[6].waiting_thread to the background thread (in
    Helmut> compute_non_keyboard_wait_mask) .  Later, the TLS connection 
receives a
    Helmut> file descriptor with the same number (6) because the sub-process 
closed
    Helmut> it already.  That would be fine, however
    Helmut> fd_callback_info[6].waiting_thread is still set to the background
    Helmut> thread.  So this time compute_non_keyboard_wait_mask doesn't 
include 6
    Helmut> in the wait mask.  Because 6 is not in the wait mask,
    Helmut> emacs_gnutls_record_check_pending will not be called and Emacs 
doesn't
    Helmut> notice the buffered output.

    Helmut> The intention in the code seems to be that clear_waiting_thread_info
    Helmut> resets fd_callback_info[6].waiting_thread to NULL, however by the 
time
    Helmut> that clear_waiting_thread_info is called, max_desc was reduced to 4,
    Helmut> because somebody closed file descriptors 5 and 6.  So
    Helmut> clear_waiting_thread_info doesn't touch fd_callback_info[6].

    Helmut> A possible solution would be to reset the .waiting_thread field in
    Helmut> delete_read_fd, like so:

    Helmut> diff --git a/src/process.c b/src/process.c
    Helmut> index 08cb810ec13..74d0bf252ab 100644
    Helmut> --- a/src/process.c
    Helmut> +++ b/src/process.c
    Helmut> @@ -513,6 +513,9 @@ delete_read_fd (int fd)
    Helmut>      {
    Helmut>        fd_callback_info[fd].func = 0;
    Helmut>        fd_callback_info[fd].data = 0;
    Helmut> +
    Helmut> +      if (fd_callback_info[fd].waiting_thread == current_thread)
    Helmut> +   fd_callback_info[fd].waiting_thread = NULL;
    Helmut>      }
    Helmut>  }

Hmm. Maybe putting it in `deactivate_process' is better (not that Iʼve
tested 😺)

Robert
--