[PATCH] posix_spawn blocks SIGCHLD in spawned processes (was: Re: master 2c79a8f 2/2: Use posix_spawn if possible.)

["Jürgen Hötzel" <juergen@hoetzel.info>]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]


Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 31 Jan 2022 22:48:33 +0200, Saulius Menkevicius <sauliusmenkevicius@fastmail.com> said:

> Try the following:
>
> diff --git a/src/callproc.c b/src/callproc.c
> index 4d3b0bb8e0..2b4e8977a3 100644
> --- a/src/callproc.c
> +++ b/src/callproc.c
> @@ -1378,6 +1378,12 @@ emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes)
>    /* Stop blocking SIGCHLD in the child.  */
>    sigset_t oldset;

^^^ This is the root-cause of the issue:
A new/invalid signal mask is introduced.
The correct oldset mask from emacs_spawn(...,const sigset_t *oldset) is
simply ignored.

>    error = pthread_sigmask (SIG_SETMASK, NULL, &oldset);
> +  if (error != 0)
> +    goto out;
> +  error = sigdelset (&oldset, SIGCHLD);
> +  if (error != 0)
> +    goto out;
> +  error = sigdelset (&oldset, SIGINT);
>    if (error != 0)
>      goto out;
>    error = posix_spawnattr_setsigmask (attributes, &oldset);
IMO the correct way to fix this issue is to use the oldset passed by
emacs_spawn (patch enclosed) just like the fork/exec implementation does.

I guess without a fix many forking commands (that don't examine and
change mask of blocked signals) will "hang" in Emacs.

"dotnet" is just a prominent one.

Jürgen


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Use correct signal oldset in posix_spawn implementation --]
[-- Type: text/x-patch, Size: 2748 bytes --]

From f691f4c304d226f99ac0377de3b6186154c0d8fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCrgen=20H=C3=B6tzel?= <juergen@archlinux.org>
Date: Fri, 4 Mar 2022 10:08:14 +0100
Subject: [PATCH] Use correct signal oldset in posix_spawn implementation

* src/callproc.c (emacs_spawn): Pass oldset parameter.
(emacs_posix_spawn_init_attributes): Use correct oldset.
(emacs_posix_spawn_init): remove intermediate function.
---
 src/callproc.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/callproc.c b/src/callproc.c
index 018c9ce690..0922e10f01 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1335,7 +1335,8 @@ emacs_posix_spawn_init_actions (posix_spawn_file_actions_t *actions,
 }
 
 static int
-emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes)
+emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes,
+				   const sigset_t *oldset)
 {
   int error = posix_spawnattr_init (attributes);
   if (error != 0)
@@ -1377,11 +1378,7 @@ emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes)
     goto out;
 
   /* Stop blocking SIGCHLD in the child.  */
-  sigset_t oldset;
-  error = pthread_sigmask (SIG_SETMASK, NULL, &oldset);
-  if (error != 0)
-    goto out;
-  error = posix_spawnattr_setsigmask (attributes, &oldset);
+  error = posix_spawnattr_setsigmask (attributes, oldset);
   if (error != 0)
     goto out;
 
@@ -1392,23 +1389,6 @@ emacs_posix_spawn_init_attributes (posix_spawnattr_t *attributes)
   return error;
 }
 
-static int
-emacs_posix_spawn_init (posix_spawn_file_actions_t *actions,
-                        posix_spawnattr_t *attributes, int std_in,
-                        int std_out, int std_err, const char *cwd)
-{
-  int error = emacs_posix_spawn_init_actions (actions, std_in,
-                                              std_out, std_err, cwd);
-  if (error != 0)
-    return error;
-
-  error = emacs_posix_spawn_init_attributes (attributes);
-  if (error != 0)
-    return error;
-
-  return 0;
-}
-
 #endif
 
 /* Start a new asynchronous subprocess.  If successful, return zero
@@ -1443,9 +1423,12 @@ emacs_spawn (pid_t *newpid, int std_in, int std_out, int std_err,
   if (use_posix_spawn)
     {
       /* Initialize optional attributes before blocking. */
-      int error
-        = emacs_posix_spawn_init (&actions, &attributes, std_in,
-                                  std_out, std_err, cwd);
+      int error = emacs_posix_spawn_init_actions (&actions, std_in,
+                                              std_out, std_err, cwd);
+      if (error != 0)
+	return error;
+
+      error = emacs_posix_spawn_init_attributes (&attributes, oldset);
       if (error != 0)
 	return error;
     }
-- 
2.35.1