On Jul 21 22:46, Takashi Yano wrote: > POSIX states system() shall be thread-safe, however, it is not in > current cygwin. This is because ch_spawn is a global and is shared > between threads. With this patch, system() uses ch_spawn_local > instead which is local variable. popen() has the same problem, so > it has been fixed in the same way. > > Addresses: https://cygwin.com/pipermail/cygwin/2025-June/258324.html > Fixes: 1fd5e000ace5 ("import winsup-2000-02-17 snapshot") > Reported-by: Takashi Yano <[email protected]> > Reviewed-by: Corinna Vinschen <[email protected]> > Signed-off-by: Takashi Yano <[email protected]> > --- > winsup/cygwin/spawn.cc | 3 ++- > winsup/cygwin/syscalls.cc | 5 +++-- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc > index fd623f4c5..c057f7ebd 100644 > --- a/winsup/cygwin/spawn.cc > +++ b/winsup/cygwin/spawn.cc > @@ -950,6 +950,7 @@ spawnve (int mode, const char *path, const char *const > *argv, > if (!envp) > envp = empty_env; > > + child_info_spawn ch_spawn_local; > switch (_P_MODE (mode)) > { > case _P_OVERLAY: > @@ -963,7 +964,7 @@ spawnve (int mode, const char *path, const char *const > *argv, > case _P_WAIT: > case _P_DETACH: > case _P_SYSTEM: > - ret = ch_spawn.worker (path, argv, envp, mode); > + ret = ch_spawn_local.worker (path, argv, envp, mode); > break; > default: > set_errno (EINVAL); > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc > index d6a2c2d3b..83a54ca05 100644 > --- a/winsup/cygwin/syscalls.cc > +++ b/winsup/cygwin/syscalls.cc > @@ -4535,8 +4535,9 @@ popen (const char *command, const char *in_type) > fcntl (stdchild, F_SETFD, stdchild_state | FD_CLOEXEC); > > /* Start a shell process to run the given command without forking. */ > - pid_t pid = ch_spawn.worker ("/bin/sh", argv, environ, _P_NOWAIT, > - __std[0], __std[1]); > + child_info_spawn ch_spawn_local; > + pid_t pid = ch_spawn_local.worker ("/bin/sh", argv, environ, _P_NOWAIT, > + __std[0], __std[1]); > > /* Reinstate the close-on-exec state */ > fcntl (stdchild, F_SETFD, stdchild_state); > -- > 2.45.1
GTG. Am I the only one who thinks it's weird that popen() maintains its own call to ch_spawn_local.worker, while system() calls spawnve with its very own _P_SYSTEM spawn mode? The obvious difference is that popen has to pass over two file descriptors, but still... Well, never mind that for now, just a stray thought. Thanks, Corinna
