From 4ae33f76d6b33ea0bedfa36050d44c88d08c2823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Mon, 6 May 2024 11:07:42 +0200 Subject: [PATCH] =?UTF-8?q?=E2=80=98system*=E2=80=99=20no=20longer=20chang?= =?UTF-8?q?es=20SIGINT=20and=20SIGQUIT=20handlers.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes . Fixes a bug whereby ‘system*’ would change the handler of SIGINT and SIGQUIT to SIG_IGN in a racy fashion, possibly competing with calls to ‘sigaction’ in Scheme in another thread. This is a followup to 527c257d6e0ad0480a859f69e9dcf3b0c7aad76e, which witch to ‘posix_spawn’, ensuring signals are properly dealt with when creating child processes. * libguile/posix.c (restore_sigaction, scm_dynwind_sigaction): Remove. (scm_system_star): Remove sigaction dynwind around call to ‘piped_process’. * NEWS: Update. Reported-by: Christopher Baines --- NEWS | 2 ++ libguile/posix.c | 38 +++----------------------------------- 2 files changed, 5 insertions(+), 35 deletions(-) diff --git a/NEWS b/NEWS index 81feccdfd..7fcba694c 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,8 @@ files. See "Random Access" in the manual for details. ** 'make-custom-port' now honors its #:conversion-strategy argument ** 'eval-string' respects #:column (previously it was set to the #:line) ** 'string->date' now allows a colon in the ISO 8601 zone offset +** 'system*' no longer fiddles with the process' signal handlers + () ** Hashing of UTF-8 symbols with non-ASCII characters avoids corruption () diff --git a/libguile/posix.c b/libguile/posix.c index f7d68200b..da60771f9 100644 --- a/libguile/posix.c +++ b/libguile/posix.c @@ -1,4 +1,4 @@ -/* Copyright 1995-2014, 2016-2019, 2021-2023 +/* Copyright 1995-2014, 2016-2019, 2021-2024 Free Software Foundation, Inc. Copyright 2021 Maxime Devos @@ -1642,27 +1642,6 @@ scm_piped_process (SCM prog, SCM args, SCM from, SCM to) } #undef FUNC_NAME -static void -restore_sigaction (SCM pair) -{ - SCM sig, handler, flags; - sig = scm_car (pair); - handler = scm_cadr (pair); - flags = scm_cddr (pair); - scm_sigaction (sig, handler, flags); -} - -static void -scm_dynwind_sigaction (int sig, SCM handler, SCM flags) -{ - SCM old, scm_sig; - scm_sig = scm_from_int (sig); - old = scm_sigaction (scm_sig, handler, flags); - scm_dynwind_unwind_handler_with_scm (restore_sigaction, - scm_cons (scm_sig, old), - SCM_F_WIND_EXPLICITLY); -} - SCM_DEFINE (scm_system_star, "system*", 0, 0, 1, (SCM args), "Execute the command indicated by @var{args}. The first element must\n" @@ -1692,17 +1671,8 @@ SCM_DEFINE (scm_system_star, "system*", 0, 0, 1, prog = scm_car (args); args = scm_cdr (args); - scm_dynwind_begin (0); - /* Make sure the child can't kill us (as per normal system call). */ - scm_dynwind_sigaction (SIGINT, - scm_from_uintptr_t ((uintptr_t) SIG_IGN), - SCM_UNDEFINED); -#ifdef SIGQUIT - scm_dynwind_sigaction (SIGQUIT, - scm_from_uintptr_t ((uintptr_t) SIG_IGN), - SCM_UNDEFINED); -#endif - + /* Note: under the hood 'posix_spawn' takes care of blocking signals + around the call to fork and resetting handlers in the child. */ err = piped_process (&pid, prog, args, SCM_UNDEFINED, SCM_UNDEFINED); if (err != 0) @@ -1718,8 +1688,6 @@ SCM_DEFINE (scm_system_star, "system*", 0, 0, 1, SCM_SYSERROR; } - scm_dynwind_end (); - return scm_from_int (status); } #undef FUNC_NAME