On Mon, 18 Aug 2025, Takashi Yano wrote:
> The class child_info_spawn has two constructors: one without arguments
> and one with two arguments. The former does not initialize any members.
> Commit 1f836c5f7394 used the latter to ensure that the local ch_spawn
> (i.e., ch_spawn_local) would be properly initialized. However, this was
> insufficient - it initialized only the base child_info members, not the
> fields specific to child_info_spawn. This led to the issue reported in
> https://cygwin.com/pipermail/cygwin/2025-August/258660.html.
>
> This patch introduces a new constructor to properly initialize member
> variable 'ev', etc., which were referred without initialization, and
> switches ch_spawn_local to use it. 'subproc_ready', which may not be
> initialized, is also initialized in the constructor of the base class
> child_info.
>
> Addresses: https://cygwin.com/pipermail/cygwin/2025-August/258660.html
> Fixes: 1f836c5f7394 ("Cygwin: spawn: Make system() thread-safe")
> Reported-by: Denis Excoffier <cyg...@denis-excoffier.org>
> Reviewed-by: Jeremy Drake <cyg...@jdrake.com>
> Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp>
LGTM.
> ---
> winsup/cygwin/local_includes/child_info.h | 3 ++-
> winsup/cygwin/sigproc.cc | 9 ++++++++-
> winsup/cygwin/spawn.cc | 2 +-
> winsup/cygwin/syscalls.cc | 2 +-
> 4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/winsup/cygwin/local_includes/child_info.h
> b/winsup/cygwin/local_includes/child_info.h
> index 2da62ffaa..25d99fa7d 100644
> --- a/winsup/cygwin/local_includes/child_info.h
> +++ b/winsup/cygwin/local_includes/child_info.h
> @@ -33,7 +33,7 @@ enum child_status
> #define EXEC_MAGIC_SIZE sizeof(child_info)
>
> /* Change this value if you get a message indicating that it is out-of-sync.
> */
> -#define CURR_CHILD_INFO_MAGIC 0xacbf4682U
> +#define CURR_CHILD_INFO_MAGIC 0x77f25a01U
>
> #include "pinfo.h"
> struct cchildren
> @@ -149,6 +149,7 @@ public:
>
> void cleanup ();
> child_info_spawn () {};
> + child_info_spawn (child_info_types);
> child_info_spawn (child_info_types, bool);
> void record_children ();
> void reattach_children ();
> diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
> index 361887981..30779cf8e 100644
> --- a/winsup/cygwin/sigproc.cc
> +++ b/winsup/cygwin/sigproc.cc
> @@ -895,7 +895,8 @@ child_info::child_info (unsigned in_cb, child_info_types
> chtype,
> msv_count (0), cb (in_cb), intro (PROC_MAGIC_GENERIC),
> magic (CHILD_INFO_MAGIC), type (chtype), cygheap (::cygheap),
> cygheap_max (::cygheap_max), flag (0), retry (child_info::retry_count),
> - rd_proc_pipe (NULL), wr_proc_pipe (NULL), sigmask (_my_tls.sigmask)
> + rd_proc_pipe (NULL), wr_proc_pipe (NULL), subproc_ready (NULL),
> + sigmask (_my_tls.sigmask)
> {
> fhandler_union_cb = sizeof (fhandler_union);
> user_h = cygwin_user_h;
> @@ -946,6 +947,12 @@ child_info_fork::child_info_fork () :
> {
> }
>
> +child_info_spawn::child_info_spawn (child_info_types chtype) :
> + child_info (sizeof *this, chtype, false), hExeced (NULL), ev (NULL),
> + sem (NULL), moreinfo (NULL)
> +{
> +}
> +
> child_info_spawn::child_info_spawn (child_info_types chtype, bool
> need_subproc_ready) :
> child_info (sizeof *this, chtype, need_subproc_ready)
> {
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 680f0fefd..71add8755 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -950,7 +950,7 @@ spawnve (int mode, const char *path, const char *const
> *argv,
> if (!envp)
> envp = empty_env;
>
> - child_info_spawn ch_spawn_local (_CH_NADA, false);
> + child_info_spawn ch_spawn_local (_CH_NADA);
> switch (_P_MODE (mode))
> {
> case _P_OVERLAY:
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 863f8f23c..1b1ff17b0 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -4535,7 +4535,7 @@ 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. */
> - child_info_spawn ch_spawn_local (_CH_NADA, false);
> + child_info_spawn ch_spawn_local (_CH_NADA);
> pid_t pid = ch_spawn_local.worker ("/bin/sh", argv, environ, _P_NOWAIT,
> __std[0], __std[1]);
>
>
