From f5b979da67595da4d91f7b7a4eb979deae6a7321 Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Mon, 12 Mar 2018 16:07:55 +0100 Subject: [PATCH 2/3] Always check GnuTLS sessions for available data * src/process.c (wait_reading_process_output): GnuTLS buffers data internally and as such there is no guarantee that a select() call on the underlying kernel socket will report available data if all data has already been buffered. Prior to GnuTLS < 2.12, lowat mode was the default which left bytes back in the kernel socket, so a select() call would work. With GnuTLS >= 2.12 (the now required version for Emacs), that default changed to non-lowat mode (and we don't set it otherwise) and was subsequently completely removed with GnuTLS >= 3.0. So, to properly handle GnuTLS sessions, we need to iterate through all channels, check for available data manually and set the concerning fds accordingly. Otherwise we might stall/delay unnecessarily or worse. This also applies to the !just_wait_proc && wait_proc case, which was previously handled improperly (only wait_proc was checked) and could cause problems if sessions did have any dependency on one another through e.g. higher up program logic and waited for one another. --- src/process.c | 77 +++++++++++++++++++---------------------------------------- 1 file changed, 24 insertions(+), 53 deletions(-) diff --git a/src/process.c b/src/process.c index 9b9b9f3550..f8fed56d5b 100644 --- a/src/process.c +++ b/src/process.c @@ -5392,60 +5392,31 @@ 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) + /* GnuTLS buffers data internally. select() will only report + available data for the underlying kernel sockets API, not + what has been buffered internally. As such, we need to loop + through all channels and check for available data 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. See - http://comments.gmane.org/gmane.emacs.devel/145074 */ - 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; + for (channel = 0; channel < FD_SETSIZE; ++channel) + if (! NILP (chan_process[channel])) + { + struct Lisp_Process *p = + XPROCESS (chan_process[channel]); + + if (just_wait_proc && p != wait_proc) + continue; + + 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); + } + } } #endif } -- 2.16.2