From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Matthias Dahl Newsgroups: gmane.emacs.devel Subject: Re: wait_reading_process_ouput hangs in certain cases (w/ patches) Date: Wed, 14 Mar 2018 10:32:29 +0100 Message-ID: <201b0871-c72b-0e0d-212b-aabff0f7fe4e@binary-island.eu> References: <709614e8-1937-07c1-f554-b453ed4f3d4a@binary-island.eu> <7550438b-9fd4-d374-e571-8bb16456cad5@cs.ucla.edu> <797d0e16-1bae-50c2-35f8-05489ffce935@binary-island.eu> <83tvugdiu5.fsf@gnu.org> <877er5s0xv.fsf@gmail.com> <4e4c72bb-295d-81e1-e4ed-cad256bca83c@binary-island.eu> <87zi3v9461.fsf@gmail.com> <87k1uy8x68.fsf@gmail.com> <6d1970af-8c5c-20ba-be09-0b9aa757d663@binary-island.eu> <13b3e003-d12b-33a7-3ebe-c07b017a7cc0@binary-island.eu> <87r2oow3uc.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------A8DED230364AAAB9C66D0AE2" X-Trace: blaine.gmane.org 1521019847 25154 195.159.176.226 (14 Mar 2018 09:30:47 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 14 Mar 2018 09:30:47 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 Cc: Paul Eggert , Eli Zaretskii , =?UTF-8?Q?andr=c3=a9s_ram=c3=adrez?= , Robert Pluim , emacs-devel@gnu.org To: Lars Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Mar 14 10:30:42 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ew2k5-0006O3-Ol for ged-emacs-devel@m.gmane.org; Wed, 14 Mar 2018 10:30:42 +0100 Original-Received: from localhost ([::1]:45097 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ew2m8-00059K-Fe for ged-emacs-devel@m.gmane.org; Wed, 14 Mar 2018 05:32:48 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:59188) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ew2lw-00058p-Sh for emacs-devel@gnu.org; Wed, 14 Mar 2018 05:32:40 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ew2lt-0003MB-M3 for emacs-devel@gnu.org; Wed, 14 Mar 2018 05:32:36 -0400 Original-Received: from ud19.udmedia.de ([194.117.254.59]:55384 helo=mail.ud19.udmedia.de) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ew2ls-0003LK-Ui for emacs-devel@gnu.org; Wed, 14 Mar 2018 05:32:33 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple; d=binary-island.eu; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type; s=k1; bh=5tMAPC3XYccQY4vA+bU4bSHa09LS UbZmwiFr7AuLrDc=; b=coRM+Oif5XLr2ZoX48JUzpr4Qp2XWtSQtL4gglGD7BIS fD+wgo1WVcy1/jcKUI/XEFcPOAnH2u5lFjR4k0UKCZ86KXUaOtkck5oDUNHPf2BL mXrOqIGunnO+XOiaeXsz/NSKmHAP2Eoc6GbM31gPadf47oWaHPB03Wb3N/eDxQw= Original-Received: (qmail 5246 invoked from network); 14 Mar 2018 10:32:31 +0100 Original-Received: from unknown (HELO ?IPv6:2a02:810b:c540:234:36aa:25b9:ca8f:d05f?) (ud19?126p1@2a02:810b:c540:234:36aa:25b9:ca8f:d05f) by mail.ud19.udmedia.de with ESMTPSA (ECDHE-RSA-AES128-GCM-SHA256 encrypted, authenticated); 14 Mar 2018 10:32:31 +0100 Openpgp: id=1E87ADA02EFE759EFC20B2D1042F47D273AA780C In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 194.117.254.59 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:223715 Archived-At: This is a multi-part message in MIME format. --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hello Lars... On 13/03/18 17:32, Lars Ingebrigtsen wrote: > Can you resend as a single patch set? I'm not 100% sure what patches to > apply... Sure. Attached you will find a combined patch for the master as well as the current emacs-26 branch. It contains all relevant fixes up until now. FYI, the relevant patches (also attached for completeness sake): Only Emacs-26 branch: 0001-Add-process-output-read-accounting.patch 0002-Fix-wait_reading_process_output-wait_proc-hang.patch This got applied to the master branch as one: 4ba32858d61eee16f17b51aca01c15211a0912f8 Emacs master and Emacs-26 branch: 0001-Fix-GnuTLS-error-handling.patch 0002-Always-check-GnuTLS-sessions-for-available-data.patch 0003-Make-xg_select-behave-more-like-pselect.patch Hope that helps. Thanks for testing. Looking forward to your results! :) Have a nice day, Matthias -- Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="0001-Add-process-output-read-accounting.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-Add-process-output-read-accounting.patch" =46rom 94e0dc26f45e1a06881b016dd26446c43d339a4d Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Tue, 24 Oct 2017 15:55:53 +0200 Subject: [PATCH 1/2] Add process output read accounting This tracks the bytes read from a process' stdin which is not used anywhere yet but required for follow-up work. * src/process.c (read_process_output): Track bytes read from a process. * src/process.h (struct Lisp_Process): Add nbytes_read to track bytes read from a process. --- src/process.c | 3 +++ src/process.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/process.c b/src/process.c index 2ec10b12ec..17fdf592ec 100644 --- a/src/process.c +++ b/src/process.c @@ -5889,6 +5889,9 @@ read_process_output (Lisp_Object proc, int channel)= return nbytes; coding->mode |=3D CODING_MODE_LAST_BLOCK; } + =20 + /* Ignore carryover, it's been added by a previous iteration already. = */ + p->nbytes_read +=3D nbytes; =20 /* Now set NBYTES how many bytes we must decode. */ nbytes +=3D carryover; diff --git a/src/process.h b/src/process.h index ab468b18c5..6464a8cc61 100644 --- a/src/process.h +++ b/src/process.h @@ -129,6 +129,8 @@ struct Lisp_Process pid_t pid; /* Descriptor by which we read from this process. */ int infd; + /* Byte-count modulo (UINTMAX_MAX + 1) for process output read from = `infd'. */ + uintmax_t nbytes_read; /* Descriptor by which we write to this process. */ int outfd; /* Descriptors that were created for this process and that need --=20 2.16.1 --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="0001-Fix-GnuTLS-error-handling.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-Fix-GnuTLS-error-handling.patch" =46rom 841d0fbe37642bc14c4bcd515cfbc91a25ba179b Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Mon, 12 Mar 2018 15:33:45 +0100 Subject: [PATCH 1/3] Fix GnuTLS error handling * src/gnutls.c (emacs_gnutls_read): All error handling should be done in `emacs_gnutls_handle_error', move handling of GNUTLS_E_UNEXPECTED_PACKET_LENGTH accordingly. We always need to set `errno' in case of an error, since later error handling (e.g. `wait_reading_process_output') depends on it and GnuTLS does not set errno by itself. We'll otherwise have random errno values which can cause erratic behavior and hangs. (emacs_gnutls_handle_error): GNUTLS_E_UNEXPECTED_PACKET_LENGTH is only returned for premature connection termination on GnuTLS < 3.0 and is otherwise a real error and should not be gobbled up. --- src/gnutls.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/gnutls.c b/src/gnutls.c index 903393fed1..e7d0d3d845 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char = *buf, ptrdiff_t nbyte) rtnval =3D gnutls_record_recv (state, buf, nbyte); if (rtnval >=3D 0) return rtnval; - else if (rtnval =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) - /* The peer closed the connection. */ - return 0; else if (emacs_gnutls_handle_error (state, rtnval)) - /* non-fatal error */ - return -1; - else { - /* a fatal error occurred */ - return 0; - } + { + /* non-fatal error */ + errno =3D EAGAIN; + return -1; + } + else + { + /* a fatal error occurred */ + errno =3D EPROTO; + return 0; + } } =20 static char const * @@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session,= int err) connection. */ # ifdef HAVE_GNUTLS3 if (err =3D=3D GNUTLS_E_PREMATURE_TERMINATION) - level =3D 3; +# else + if (err =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) # endif + level =3D 3; =20 GNUTLS_LOG2 (level, max_log_level, "fatal error:", str); ret =3D false; --=20 2.16.2 --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="0002-Always-check-GnuTLS-sessions-for-available-data.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0002-Always-check-GnuTLS-sessions-for-available-data.patch" =46rom 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 >=3D 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 >=3D 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 */ =20 #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 =3D=3D 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 >=3D 0) { - fd_set tls_available; - int set =3D 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 =3D 0; channel < FD_SETSIZE; ++channel) - if (! NILP (chan_process[channel])) - { - struct Lisp_Process *p =3D - 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 =3D=3D 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 =3D 1; - eassert (0 <=3D wait_proc->infd); - /* Set to Available. */ - FD_SET (wait_proc->infd, &tls_available); - set++; - } - } - if (set) - Available =3D tls_available; + for (channel =3D 0; channel < FD_SETSIZE; ++channel) + if (! NILP (chan_process[channel])) + { + struct Lisp_Process *p =3D + XPROCESS (chan_process[channel]); + + if (just_wait_proc && p !=3D wait_proc) + continue; + + if (p && p->gnutls_p && p->gnutls_state + && ((emacs_gnutls_record_check_pending + (p->gnutls_state)) + > 0)) + { + nfds++; + eassert (p->infd =3D=3D channel); + FD_SET (p->infd, &Available); + } + } } #endif } --=20 2.16.2 --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="0002-Fix-wait_reading_process_output-wait_proc-hang.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0002-Fix-wait_reading_process_output-wait_proc-hang.patch" =46rom b9c05bbfb4559b21deb0ea4e156430dedb60ce41 Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Tue, 6 Feb 2018 15:24:15 +0100 Subject: [PATCH 2/2] Fix wait_reading_process_output wait_proc hang * src/process.c (wait_reading_process_output): If called recursively through timers and/or process filters via accept-process-output, it is possible that the output of wait_proc has already been read by one of those recursive calls, leaving the original call hanging forever if no further output arrives through that fd and no timeout has been set. Fix that by using the process read accounting to keep track of how many bytes have been read and use that as a condition to break out of the infinite loop and return to the caller as well as to calculate the proper return value (if a wait_proc is given that is). --- src/process.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/process.c b/src/process.c index 17fdf592ec..0abbd5fa8e 100644 --- a/src/process.c +++ b/src/process.c @@ -5006,6 +5006,7 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, struct timespec got_output_end_time =3D invalid_timespec (); enum { MINIMUM =3D -1, TIMEOUT, INFINITY } wait; int got_some_output =3D -1; + uintmax_t prev_wait_proc_nbytes_read =3D wait_proc ? wait_proc->nbytes= _read : 0; #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS bool retry_for_async; #endif @@ -5460,6 +5461,8 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, if (nfds =3D=3D 0) { /* Exit the main loop if we've passed the requested timeout, + or have read some bytes from our wait_proc (either directly= + in this call or indirectly through timers / process filters= ), or aren't skipping processes and got some output and haven't lowered our timeout due to timers or SIGIO and have waited a long amount of time due to repeated @@ -5467,7 +5470,9 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, struct timespec huge_timespec =3D make_timespec (TYPE_MAXIMUM (time_t), 2 * TIMESPEC_RESOLUTION);= struct timespec cmp_time =3D huge_timespec; - if (wait < TIMEOUT) + if (wait < TIMEOUT + || (wait_proc + && wait_proc->nbytes_read !=3D prev_wait_proc_nbytes_r= ead)) break; if (wait =3D=3D TIMEOUT) cmp_time =3D end_time; @@ -5772,6 +5777,15 @@ wait_reading_process_output (intmax_t time_limit, = int nsecs, int read_kbd, maybe_quit (); } =20 + /* Timers and/or process filters that we have run could have themselve= s called + `accept-process-output' (and by that indirectly this function), thu= s + possibly reading some (or all) output of wait_proc without us notic= ing it. + This could potentially lead to an endless wait (dealt with earlier = in the + function) and/or a wrong return value (dealt with here). */ + if (wait_proc && wait_proc->nbytes_read !=3D prev_wait_proc_nbytes_rea= d) + got_some_output =3D min (INT_MAX, (wait_proc->nbytes_read + - prev_wait_proc_nbytes_read)); + return got_some_output; } =0C --=20 2.16.1 --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="0003-Make-xg_select-behave-more-like-pselect.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0003-Make-xg_select-behave-more-like-pselect.patch" =46rom 2f44ef364bc320dfd73febc51f0c0da862db49b1 Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Tue, 13 Mar 2018 15:35:16 +0100 Subject: [PATCH 3/3] Make xg_select() behave more like pselect() * src/xgselect.c (xg_select): If no file descriptors have data ready, pselect() clears the passed in fd sets whereas xg_select() does not which caused Bug#21337 for `wait_reading_process_output'. Clear the passed in sets if no fds are ready but leave them untouched if pselect() returns an error -- just like pselect() does itself. --- src/xgselect.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/xgselect.c b/src/xgselect.c index fedd3127ef..f68982143e 100644 --- a/src/xgselect.c +++ b/src/xgselect.c @@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, = fd_set *efds, ++retval; } } + else if (nfds =3D=3D 0) + { + // pselect() clears the file descriptor sets if no fd is ready (bu= t + // not if an error occurred), so should we to be compatible. (Bug#= 21337) + if (rfds) FD_ZERO (rfds); + if (wfds) FD_ZERO (wfds); + if (efds) FD_ZERO (efds); + } =20 /* If Gtk+ is in use eventually gtk_main_iteration will be called, unless retval is zero. */ --=20 2.16.2 --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="emacs-emacs26-combined.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="emacs-emacs26-combined.patch" diff --git a/src/gnutls.c b/src/gnutls.c index 903393fed1..e7d0d3d845 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char = *buf, ptrdiff_t nbyte) rtnval =3D gnutls_record_recv (state, buf, nbyte); if (rtnval >=3D 0) return rtnval; - else if (rtnval =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) - /* The peer closed the connection. */ - return 0; else if (emacs_gnutls_handle_error (state, rtnval)) - /* non-fatal error */ - return -1; - else { - /* a fatal error occurred */ - return 0; - } + { + /* non-fatal error */ + errno =3D EAGAIN; + return -1; + } + else + { + /* a fatal error occurred */ + errno =3D EPROTO; + return 0; + } } =20 static char const * @@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session,= int err) connection. */ # ifdef HAVE_GNUTLS3 if (err =3D=3D GNUTLS_E_PREMATURE_TERMINATION) - level =3D 3; +# else + if (err =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) # endif + level =3D 3; =20 GNUTLS_LOG2 (level, max_log_level, "fatal error:", str); ret =3D false; diff --git a/src/process.c b/src/process.c index b201e9b6ac..c1116c1deb 100644 --- a/src/process.c +++ b/src/process.c @@ -5005,6 +5005,7 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, struct timespec got_output_end_time =3D invalid_timespec (); enum { MINIMUM =3D -1, TIMEOUT, INFINITY } wait; int got_some_output =3D -1; + uintmax_t prev_wait_proc_nbytes_read =3D wait_proc ? wait_proc->nbytes= _read : 0; #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS bool retry_for_async; #endif @@ -5390,60 +5391,31 @@ wait_reading_process_output (intmax_t time_limit,= int nsecs, int read_kbd, #endif /* !HAVE_GLIB */ =20 #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 =3D=3D 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 >=3D 0) { - fd_set tls_available; - int set =3D 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 =3D 0; channel < FD_SETSIZE; ++channel) - if (! NILP (chan_process[channel])) - { - struct Lisp_Process *p =3D - 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 =3D=3D 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 =3D 1; - eassert (0 <=3D wait_proc->infd); - /* Set to Available. */ - FD_SET (wait_proc->infd, &tls_available); - set++; - } - } - if (set) - Available =3D tls_available; + for (channel =3D 0; channel < FD_SETSIZE; ++channel) + if (! NILP (chan_process[channel])) + { + struct Lisp_Process *p =3D + XPROCESS (chan_process[channel]); + + if (just_wait_proc && p !=3D wait_proc) + continue; + + if (p && p->gnutls_p && p->gnutls_state + && ((emacs_gnutls_record_check_pending + (p->gnutls_state)) + > 0)) + { + nfds++; + eassert (p->infd =3D=3D channel); + FD_SET (p->infd, &Available); + } + } } #endif } @@ -5459,6 +5431,8 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, if (nfds =3D=3D 0) { /* Exit the main loop if we've passed the requested timeout, + or have read some bytes from our wait_proc (either directly= + in this call or indirectly through timers / process filters= ), or aren't skipping processes and got some output and haven't lowered our timeout due to timers or SIGIO and have waited a long amount of time due to repeated @@ -5466,7 +5440,9 @@ wait_reading_process_output (intmax_t time_limit, i= nt nsecs, int read_kbd, struct timespec huge_timespec =3D make_timespec (TYPE_MAXIMUM (time_t), 2 * TIMESPEC_RESOLUTION);= struct timespec cmp_time =3D huge_timespec; - if (wait < TIMEOUT) + if (wait < TIMEOUT + || (wait_proc + && wait_proc->nbytes_read !=3D prev_wait_proc_nbytes_r= ead)) break; if (wait =3D=3D TIMEOUT) cmp_time =3D end_time; @@ -5781,6 +5757,15 @@ wait_reading_process_output (intmax_t time_limit, = int nsecs, int read_kbd, maybe_quit (); } =20 + /* Timers and/or process filters that we have run could have themselve= s called + `accept-process-output' (and by that indirectly this function), thu= s + possibly reading some (or all) output of wait_proc without us notic= ing it. + This could potentially lead to an endless wait (dealt with earlier = in the + function) and/or a wrong return value (dealt with here). */ + if (wait_proc && wait_proc->nbytes_read !=3D prev_wait_proc_nbytes_rea= d) + got_some_output =3D min (INT_MAX, (wait_proc->nbytes_read + - prev_wait_proc_nbytes_read)); + return got_some_output; } =0C @@ -5899,6 +5884,9 @@ read_process_output (Lisp_Object proc, int channel)= coding->mode |=3D CODING_MODE_LAST_BLOCK; } =20 + /* Ignore carryover, it's been added by a previous iteration already. = */ + p->nbytes_read +=3D nbytes; + /* Now set NBYTES how many bytes we must decode. */ nbytes +=3D carryover; =20 diff --git a/src/process.h b/src/process.h index ab468b18c5..6464a8cc61 100644 --- a/src/process.h +++ b/src/process.h @@ -129,6 +129,8 @@ struct Lisp_Process pid_t pid; /* Descriptor by which we read from this process. */ int infd; + /* Byte-count modulo (UINTMAX_MAX + 1) for process output read from = `infd'. */ + uintmax_t nbytes_read; /* Descriptor by which we write to this process. */ int outfd; /* Descriptors that were created for this process and that need diff --git a/src/xgselect.c b/src/xgselect.c index fedd3127ef..f68982143e 100644 --- a/src/xgselect.c +++ b/src/xgselect.c @@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, = fd_set *efds, ++retval; } } + else if (nfds =3D=3D 0) + { + // pselect() clears the file descriptor sets if no fd is ready (bu= t + // not if an error occurred), so should we to be compatible. (Bug#= 21337) + if (rfds) FD_ZERO (rfds); + if (wfds) FD_ZERO (wfds); + if (efds) FD_ZERO (efds); + } =20 /* If Gtk+ is in use eventually gtk_main_iteration will be called, unless retval is zero. */ --------------A8DED230364AAAB9C66D0AE2 Content-Type: text/x-patch; name="emacs-master-combined.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="emacs-master-combined.patch" diff --git a/src/gnutls.c b/src/gnutls.c index 903393fed1..e7d0d3d845 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -708,16 +708,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char = *buf, ptrdiff_t nbyte) rtnval =3D gnutls_record_recv (state, buf, nbyte); if (rtnval >=3D 0) return rtnval; - else if (rtnval =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) - /* The peer closed the connection. */ - return 0; else if (emacs_gnutls_handle_error (state, rtnval)) - /* non-fatal error */ - return -1; - else { - /* a fatal error occurred */ - return 0; - } + { + /* non-fatal error */ + errno =3D EAGAIN; + return -1; + } + else + { + /* a fatal error occurred */ + errno =3D EPROTO; + return 0; + } } =20 static char const * @@ -756,8 +758,10 @@ emacs_gnutls_handle_error (gnutls_session_t session,= int err) connection. */ # ifdef HAVE_GNUTLS3 if (err =3D=3D GNUTLS_E_PREMATURE_TERMINATION) - level =3D 3; +# else + if (err =3D=3D GNUTLS_E_UNEXPECTED_PACKET_LENGTH) # endif + level =3D 3; =20 GNUTLS_LOG2 (level, max_log_level, "fatal error:", str); ret =3D false; 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 */ =20 #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 =3D=3D 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 >=3D 0) { - fd_set tls_available; - int set =3D 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 =3D 0; channel < FD_SETSIZE; ++channel) - if (! NILP (chan_process[channel])) - { - struct Lisp_Process *p =3D - 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 =3D=3D 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 =3D 1; - eassert (0 <=3D wait_proc->infd); - /* Set to Available. */ - FD_SET (wait_proc->infd, &tls_available); - set++; - } - } - if (set) - Available =3D tls_available; + for (channel =3D 0; channel < FD_SETSIZE; ++channel) + if (! NILP (chan_process[channel])) + { + struct Lisp_Process *p =3D + XPROCESS (chan_process[channel]); + + if (just_wait_proc && p !=3D wait_proc) + continue; + + if (p && p->gnutls_p && p->gnutls_state + && ((emacs_gnutls_record_check_pending + (p->gnutls_state)) + > 0)) + { + nfds++; + eassert (p->infd =3D=3D channel); + FD_SET (p->infd, &Available); + } + } } #endif } diff --git a/src/xgselect.c b/src/xgselect.c index fedd3127ef..f68982143e 100644 --- a/src/xgselect.c +++ b/src/xgselect.c @@ -143,6 +143,14 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, = fd_set *efds, ++retval; } } + else if (nfds =3D=3D 0) + { + // pselect() clears the file descriptor sets if no fd is ready (bu= t + // not if an error occurred), so should we to be compatible. (Bug#= 21337) + if (rfds) FD_ZERO (rfds); + if (wfds) FD_ZERO (wfds); + if (efds) FD_ZERO (efds); + } =20 /* If Gtk+ is in use eventually gtk_main_iteration will be called, unless retval is zero. */ --------------A8DED230364AAAB9C66D0AE2--