Re: kern/59175: posix_spawn hang, hanging other process too

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/59175; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: wiz%NetBSD.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/59175: posix_spawn hang, hanging other process too
Date: Thu, 13 Mar 2025 21:54:58 +0000

 This is a multi-part message in MIME format.
 --=_ml7q+P8YFLTA7qSAPcDHHBHCSIXmg0yj
 
 The attached patch should fix one of the problems that led into this.
 However, the hypothesis justifying it doesn't explain all the symptoms
 you're seeing, so there's more to investigate -- still not clear why
 sysctl_kern_proc_args is repeatedly trapping.
 
 --=_ml7q+P8YFLTA7qSAPcDHHBHCSIXmg0yj
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr59175-spawnwaitcondvarabuse"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr59175-spawnwaitcondvarabuse.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1741902749 0
 #      Thu Mar 13 21:52:29 2025 +0000
 # Branch trunk
 # Node ID 6be52ee843dae2b1bfd85f8c4feb856dc4229fb2
 # Parent  e446c5e13d047e0488bd251fe8405018b430338c
 # EXP-Topic riastradh-pr59175-spawnwait
 posix_spawn(2): Fix race between parent and child.
 
 This was an embarrassing misuse of condition variables.
 
 PR kern/59175: posix_spawn hang, hanging other process too
 
 (Doesn't resolve everything about the symptoms but this was obviously
 a bug that needed fixing.)
 
 diff -r e446c5e13d04 -r 6be52ee843da sys/kern/kern_exec.c
 --- a/sys/kern/kern_exec.c	Tue Mar 11 17:46:12 2025 +0000
 +++ b/sys/kern/kern_exec.c	Thu Mar 13 21:52:29 2025 +0000
 @@ -281,6 +281,7 @@ struct spawn_exec_data {
  	kcondvar_t		sed_cv_child_ready;
  	kmutex_t		sed_mtx_child;
  	int			sed_error;
 +	bool			sed_child_ready;
  	volatile uint32_t	sed_refcnt;
  };
 =20
 @@ -2347,6 +2348,9 @@ spawn_return(void *arg)
  	    && rw_tryenter(&exec_lock, RW_READER)) {
  		parent_is_waiting =3D false;
  		mutex_enter(&spawn_data->sed_mtx_child);
 +		KASSERT(!spawn_data->sed_child_ready);
 +		spawn_data->sed_error =3D 0;
 +		spawn_data->sed_child_ready =3D true;
  		cv_signal(&spawn_data->sed_cv_child_ready);
  		mutex_exit(&spawn_data->sed_mtx_child);
  	}
 @@ -2376,6 +2380,9 @@ spawn_return(void *arg)
 =20
  	if (parent_is_waiting) {
  		mutex_enter(&spawn_data->sed_mtx_child);
 +		KASSERT(!spawn_data->sed_child_ready);
 +		spawn_data->sed_error =3D 0;
 +		spawn_data->sed_child_ready =3D true;
  		cv_signal(&spawn_data->sed_cv_child_ready);
  		mutex_exit(&spawn_data->sed_mtx_child);
  	}
 @@ -2409,7 +2416,9 @@ spawn_return(void *arg)
  	if (parent_is_waiting) {
  		/* pass error to parent */
  		mutex_enter(&spawn_data->sed_mtx_child);
 +		KASSERT(!spawn_data->sed_child_ready);
  		spawn_data->sed_error =3D error;
 +		spawn_data->sed_child_ready =3D true;
  		cv_signal(&spawn_data->sed_cv_child_ready);
  		mutex_exit(&spawn_data->sed_mtx_child);
  	} else {
 @@ -2814,7 +2823,10 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
  	mutex_exit(p2->p_lock);
  	mutex_exit(&proc_lock);
 =20
 -	cv_wait(&spawn_data->sed_cv_child_ready, &spawn_data->sed_mtx_child);
 +	while (!spawn_data->sed_child_ready) {
 +		cv_wait(&spawn_data->sed_cv_child_ready,
 +		    &spawn_data->sed_mtx_child);
 +	}
  	error =3D spawn_data->sed_error;
  	mutex_exit(&spawn_data->sed_mtx_child);
  	spawn_exec_data_release(spawn_data);
 
 --=_ml7q+P8YFLTA7qSAPcDHHBHCSIXmg0yj--