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: Tue, 21 Nov 2017 15:44:10 +0100 Message-ID: <709614e8-1937-07c1-f554-b453ed4f3d4a@binary-island.eu> References: <83r2tetf90.fsf@gnu.org> <5150d198-8dd3-9cf4-5914-b7e945294452@binary-island.eu> <83tvy7s6wi.fsf@gnu.org> <83inemrqid.fsf@gnu.org> <398f8d17-b727-d5d6-4a31-772448c7ca0d@binary-island.eu> <56e722a6-95a4-0e42-185c-f26845d4f4bf@binary-island.eu> <21237e45-a353-92f9-01ec-7b51640d2031@cs.ucla.edu> <83vaickfu2.fsf@gnu.org> <83tvxwkexg.fsf@gnu.org> <03261534-6bf5-1a5d-915f-d3c55aaa35e9@binary-island.eu> <206ebefa-7583-f049-140c-c8fd041b0719@cs.ucla.edu> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------6EEED88BBDA7280077C355FE" X-Trace: blaine.gmane.org 1511276114 10621 195.159.176.226 (21 Nov 2017 14:55:14 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 21 Nov 2017 14:55:14 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 Cc: emacs-devel@gnu.org To: Paul Eggert , Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Nov 21 15:55:07 2017 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 1eH9x0-000202-JX for ged-emacs-devel@m.gmane.org; Tue, 21 Nov 2017 15:55:04 +0100 Original-Received: from localhost ([::1]:34902 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eH9x8-0006GK-1W for ged-emacs-devel@m.gmane.org; Tue, 21 Nov 2017 09:55:10 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54311) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eH9md-0005l8-7L for emacs-devel@gnu.org; Tue, 21 Nov 2017 09:44:20 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eH9ma-0008Jm-3r for emacs-devel@gnu.org; Tue, 21 Nov 2017 09:44:19 -0500 Original-Received: from ud19.udmedia.de ([194.117.254.59]:53396 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 1eH9mZ-0008IT-LZ for emacs-devel@gnu.org; Tue, 21 Nov 2017 09:44:16 -0500 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=RJBpXxBzIwxwdrcbrexrt8okICqt 0/RtwPOX9m4k6CA=; b=mQYB1pdTyU015ZfKCucJo2u35G4PsBkqxsKqAsPlz9Hg zypjNS/EeFWaQOXB9BDoZjh5i4cfh55tRcaOKm78GeIx+EUmfNuOjI/P8DHC3Z8A qf73aJoh2nIVNBaTRRqwiMKWQuyWJaPVvLzskTilRHbBSBbBY7dZ1MqhTpUFNz4= Original-Received: (qmail 9630 invoked from network); 21 Nov 2017 15:44:12 +0100 Original-Received: from ip5f58867b.dynamic.kabel-deutschland.de (HELO ?10.0.0.20?) (ud19?126p1@95.88.134.123) by mail.ud19.udmedia.de with ESMTPSA (ECDHE-RSA-AES128-GCM-SHA256 encrypted, authenticated); 21 Nov 2017 15:44:12 +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:220327 Archived-At: This is a multi-part message in MIME format. --------------6EEED88BBDA7280077C355FE Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Hello Eli and Paul, attached you find the updated patches which have all the discussed changes and fixes. If there is anything else, please let me know. Thanks again for all the patience and valuable feedback. Have a nice day, Matthias -- Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu --------------6EEED88BBDA7280077C355FE 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 94cd9ac3867305184f310dbf411729c59897c2c5 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's 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 infd_num_bytes_read to track bytes read from a process. --- src/process.c | 4 ++++ src/process.h | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/process.c b/src/process.c index fc46e74332..ab023457bd 100644 --- a/src/process.c +++ b/src/process.c @@ -5886,6 +5886,10 @@ read_process_output (Lisp_Object proc, int channel= ) nbytes +=3D buffered && nbytes <=3D 0; } =20 + /* Don't count carryover as those bytes have already been count by + a previous iteration. */ + p->infd_num_bytes_read +=3D nbytes; + p->decoding_carryover =3D 0; =20 /* At this point, NBYTES holds number of bytes just received diff --git a/src/process.h b/src/process.h index 5670f44736..96c19fcf81 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 infd_num_bytes_read; /* Descriptor by which we write to this process. */ int outfd; /* Descriptors that were created for this process and that need --=20 2.15.0 --------------6EEED88BBDA7280077C355FE Content-Type: text/x-patch; name="0002-src-process.c-wait_reading_process_output-Fix-wait_p.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename*0="0002-src-process.c-wait_reading_process_output-Fix-wait_p.pa"; filename*1="tch" =46rom 1bbe69611bb4db8bd4149d57cfa5be548ee64c9d Mon Sep 17 00:00:00 2001 From: Matthias Dahl Date: Tue, 24 Oct 2017 15:56:47 +0200 Subject: [PATCH 2/2] * src/process.c (wait_reading_process_output): Fix wait_proc hang. If called recursively (through timers or process filters by the means of 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 specified. Implement proper checks by taking advantage of the process output read accounting. --- src/process.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/process.c b/src/process.c index ab023457bd..b75ac171a1 100644 --- a/src/process.c +++ b/src/process.c @@ -5003,6 +5003,8 @@ 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 initial_wait_proc_num_bytes_read =3D (wait_proc) ? + wait_proc->infd_num_bytes= _read : 0; #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS bool retry_for_async; #endif @@ -5161,6 +5163,19 @@ wait_reading_process_output (intmax_t time_limit, = int nsecs, int read_kbd, && requeued_events_pending_p ()) break; =20 + /* Timers could have called `accept-process-output', thus read= ing the output + of wait_proc while we (in the worst case) wait endlessly fo= r it to become + available later. So we need to check if data has been read = and break out + early if that is so since our job has been fulfilled. */ + if (wait_proc + && wait_proc->infd_num_bytes_read !=3D initial_wait_proc_n= um_bytes_read) + { + /* Make sure we don't overflow signed got_some_output. + Calculating bytes read is modulo (UINTMAX_MAX + 1) and = won't overflow. */ + got_some_output =3D min(INT_MAX, (wait_proc->infd_num_byte= s_read + - initial_wait_proc_num_by= tes_read)); + } + /* This is so a breakpoint can be put here. */ if (!timespec_valid_p (timer_delay)) wait_reading_process_output_1 (); @@ -5606,7 +5621,20 @@ wait_reading_process_output (intmax_t time_limit, = int nsecs, int read_kbd, buffered-ahead character if we have one. */ =20 nread =3D read_process_output (proc, channel); - if ((!wait_proc || wait_proc =3D=3D XPROCESS (proc)) + + /* In case a filter was run that called `accept-process-ou= tput', it is + possible that the output from wait_proc was already rea= d, leaving us + waiting for it endlessly (if no timeout was specified).= Thus, we need + to check if data was already read. */ + if (wait_proc + && wait_proc->infd_num_bytes_read !=3D initial_wait_pr= oc_num_bytes_read) + { + /* Make sure we don't overflow signed got_some_output.= + Calculating bytes read is modulo (UINTMAX_MAX + 1) = and won't overflow. */ + got_some_output =3D min(INT_MAX, (wait_proc->infd_num_= bytes_read + - initial_wait_proc_nu= m_bytes_read)); + } + else if ((!wait_proc || wait_proc =3D=3D XPROCESS (proc)) && got_some_output < nread) got_some_output =3D nread; if (nread > 0) --=20 2.15.0 --------------6EEED88BBDA7280077C355FE--