[Guile-commits] 01/08: ‘system*’ no longer changes SIGINT and SIGQUIT handlers.

[Ludovic Courtès]

civodul pushed a commit to branch main
in repository guile.

commit 4ae33f76d6b33ea0bedfa36050d44c88d08c2823
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Mon May 6 11:07:42 2024 +0200

    ‘system*’ no longer changes SIGINT and SIGQUIT handlers.
    
    Fixes <https://bugs.gnu.org/70144>.
    
    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 <mail@cbaines.net>
---
 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
+   (<https://bugs.gnu.org/70144>)
 ** Hashing of UTF-8 symbols with non-ASCII characters avoids corruption
    (<https://bugs.gnu.org/56413>)
 
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 <maximedevos@telenet.be>
 
@@ -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