From unknown Thu Sep 18 22:55:33 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79436: [PATCH] Use up-to-date time in wait_reading_process_output Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: eggert@cs.ucla.edu, dmitry@gutov.dev, bug-gnu-emacs@gnu.org Resent-Date: Fri, 12 Sep 2025 15:40:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 79436 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: 79436@debbugs.gnu.org Cc: Paul Eggert , Dmitry Gutov X-Debbugs-Original-To: bug-gnu-emacs@gnu.org X-Debbugs-Original-Xcc: Paul Eggert , Dmitry Gutov Received: via spool by submit@debbugs.gnu.org id=B.175769154624201 (code B ref -1); Fri, 12 Sep 2025 15:40:03 +0000 Received: (at submit) by debbugs.gnu.org; 12 Sep 2025 15:39:06 +0000 Received: from localhost ([127.0.0.1]:50666 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ux5rd-0006IG-5u for submit@debbugs.gnu.org; Fri, 12 Sep 2025 11:39:05 -0400 Received: from lists.gnu.org ([2001:470:142::17]:56448) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1ux5rT-0006Fq-T4 for submit@debbugs.gnu.org; Fri, 12 Sep 2025 11:39:01 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ux5rB-0003yP-Rk for bug-gnu-emacs@gnu.org; Fri, 12 Sep 2025 11:38:40 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ux5r6-0006IB-2c for bug-gnu-emacs@gnu.org; Fri, 12 Sep 2025 11:38:37 -0400 From: Spencer Baugh Date: Fri, 12 Sep 2025 11:38:29 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1757691509; bh=Qg5hUw1bDjc626U4ItFk/PeG+6k3CCwMoB7NB8Y7vs8=; h=From:To:Subject:Date; b=t1qXPOA/KYCtr4d0Mch+PgAfnI805XeKFfcnMr06b1a1M+bCAv+ZD2GdWioo7GLM+ aStbD2FQZ50b9RoP3HZRTLyYDQ+kZWCoxRsWgFrodBeIS7r7PYKJWqgOsMPI7ohrlH AjDkemIXkjoF5yNqd/gw34KJRw9sAXVkQlhynlmVAtAYWfzxVSSyPBRYNxXtYwFAAe t7d2WJbI7fxwuM3k8vs3aTWIW1PlDmVQyrcjNoQNfITlwfSSLF+6rT7TubrZ0K4vfT lBBOPo0Jrh/Wvk9IlW0z1a3HW4+2JFVhOgW5Uz2C6dUhutFfFQ09UfFeJ731tIv71/ H7A0O6OxrQ5jw== Received-SPF: pass client-ip=64.215.233.18; envelope-from=sbaugh@janestreet.com; helo=mxout5.mail.janestreet.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.9 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.1 (/) --=-=-= Content-Type: text/plain Tags: patch Compile the following C program, close_and_sit.c: #include #include void main() { fclose(stdout); fclose(stderr); while (1) sleep(1); } Then run this snippet of Lisp, updating the executable file name appropriately: (make-process :name "close_and_sit" :command '("./close_and_sit") :connection-type 'pipe) (make-process :name "true" :command '("true")) (message "accept-process-output") (accept-process-output nil 1) This will cause Emacs to hang; accept-process-output will never time out. There are two bugs which combine to cause this. The first bug is in our timeout handling. The attached patch resolves that, which causes accept-process-output to time out properly and fixes the bug. Paul, this patch partially reverts a change you made to cache current_timespec. Does it seem right to you? The second bug is that when a child process closes stdout/stderr, we continue watching the pipe, generating spurious events. (We should still be timing out despite the spurious events - real events could also cause this hang, this bug just makes it easier to trigger the hang.) I'm still working on a patch to fix the second bug. --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Use-up-to-date-time-in-wait_reading_process_output.patch >From 338810c5ac4f9831bf1cff23a965ac0d460f9f7f Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Fri, 12 Sep 2025 11:18:28 -0400 Subject: [PATCH] Use up-to-date time in wait_reading_process_output In ad23626030f0541e761f683661b51152128fb0b5 we changed to call current_timespec less frequently, and instead cached it in "now". However, this is buggy: we only updated now when timeout was non-zero, but timeout can be set temporarily to zero in several places in wait_reading_process_output (such as when checking for process status changes), which would cause us to never update "now" and therefore never detect that a timeout happened. Also, this caching is wrong even in principle: since we call Lisp code from wait_reading_process_output, substantial amounts of time can pass, and we'd be left using an outdated "now" and incorrectly not time out. Also, the caching isn't a useful performance optimization: current_timespec is fast. So just get rid of it. * src/process.c (wait_reading_process_output): Stop caching current_timespec. --- src/process.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/process.c b/src/process.c index db7034fa98f..00658adcb28 100644 --- a/src/process.c +++ b/src/process.c @@ -5347,9 +5347,6 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, #endif specpdl_ref count = SPECPDL_INDEX (); - /* Close to the current time if known, an invalid timespec otherwise. */ - struct timespec now = invalid_timespec (); - eassert (wait_proc == NULL || NILP (wait_proc->thread) || XTHREAD (wait_proc->thread) == current_thread); @@ -5374,8 +5371,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, else if (time_limit > 0 || nsecs > 0) { wait = TIMEOUT; - now = current_timespec (); - end_time = timespec_add (now, make_timespec (time_limit, nsecs)); + end_time = timespec_add (current_timespec (), make_timespec (time_limit, nsecs)); } else wait = FOREVER; @@ -5461,8 +5457,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, /* Exit if already run out. */ if (wait == TIMEOUT) { - if (!timespec_valid_p (now)) - now = current_timespec (); + struct timespec now = current_timespec (); if (timespec_cmp (end_time, now) <= 0) break; timeout = timespec_sub (end_time, now); @@ -5718,9 +5713,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, && timespec_valid_p (timer_delay) && timespec_cmp (timer_delay, timeout) < 0) { - if (!timespec_valid_p (now)) - now = current_timespec (); - struct timespec timeout_abs = timespec_add (now, timeout); + struct timespec timeout_abs = timespec_add (current_timespec (), timeout); if (!timespec_valid_p (got_output_end_time) || timespec_cmp (timeout_abs, got_output_end_time) < 0) got_output_end_time = timeout_abs; @@ -5729,10 +5722,6 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, else got_output_end_time = invalid_timespec (); - /* NOW can become inaccurate if time can pass during pselect. */ - if (timeout.tv_sec > 0 || timeout.tv_nsec > 0) - now = invalid_timespec (); - #if defined HAVE_GETADDRINFO_A || defined HAVE_GNUTLS if (retry_for_async && (timeout.tv_sec > 0 || timeout.tv_nsec > ASYNC_RETRY_NSEC)) @@ -5902,8 +5891,7 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, } if (timespec_cmp (cmp_time, huge_timespec) < 0) { - now = current_timespec (); - if (timespec_cmp (cmp_time, now) <= 0) + if (timespec_cmp (cmp_time, current_timespec ()) <= 0) break; } } -- 2.43.7 --=-=-=--