The PID_STOPPED flag in _ponfo::process_state is sometimes accidentally cleared due to a race condition when modifying it with the "|=" or "&=" operators. This patch uses InterlockedOr/And() instead to avoid the race condition.
Addresses: https://cygwin.com/pipermail/cygwin/2025-February/257473.html Fixes: 1fd5e000ace55 ("import winsup-2000-02-17 snapshot") Reported-by: Christian Franke <christian.fra...@t-online.de> Reviewed-by: Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> --- winsup/cygwin/exceptions.cc | 6 +++--- winsup/cygwin/fork.cc | 5 +++-- winsup/cygwin/local_includes/pinfo.h | 4 ++-- winsup/cygwin/pinfo.cc | 11 ++++++----- winsup/cygwin/signal.cc | 6 ++++-- winsup/cygwin/sigproc.cc | 2 +- winsup/cygwin/spawn.cc | 6 +++--- 7 files changed, 22 insertions(+), 18 deletions(-) diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index c6e82b6c5..45c71cbdf 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -882,7 +882,7 @@ sig_handle_tty_stop (int sig, siginfo_t *, void *) /* Silently ignore attempts to suspend if there is no accommodating cygwin parent to deal with this behavior. */ if (!myself->cygstarted) - myself->process_state &= ~PID_STOPPED; + InterlockedAnd ((LONG *) &myself->process_state, ~PID_STOPPED); else { _my_tls.incyg = 1; @@ -948,7 +948,7 @@ _cygtls::interrupt_setup (siginfo_t& si, void *handler, struct sigaction& siga) if (handler == sig_handle_tty_stop) { myself->stopsig = 0; - myself->process_state |= PID_STOPPED; + InterlockedOr ((LONG *) &myself->process_state, PID_STOPPED); } infodata = si; @@ -1435,7 +1435,7 @@ _cygtls::handle_SIGCONT () yield (); myself->stopsig = 0; - myself->process_state &= ~PID_STOPPED; + InterlockedAnd ((LONG *) &myself->process_state, ~PID_STOPPED); /* Clear pending stop signals */ sig_clear (SIGSTOP, false); diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 41a533705..783971b76 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -678,8 +678,9 @@ dofork (void **proc, bool *with_forkables) if (ischild) { - myself->process_state |= PID_ACTIVE; - myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED); + InterlockedOr ((LONG *) &myself->process_state, PID_ACTIVE); + InterlockedAnd ((LONG *) &myself->process_state, + ~(PID_INITIALIZING | PID_EXITED | PID_REAPED)); } else if (res < 0) { diff --git a/winsup/cygwin/local_includes/pinfo.h b/winsup/cygwin/local_includes/pinfo.h index 4de0f80dd..3086f9941 100644 --- a/winsup/cygwin/local_includes/pinfo.h +++ b/winsup/cygwin/local_includes/pinfo.h @@ -271,9 +271,9 @@ public: push_process_state (int add_flag) { flag = add_flag; - myself->process_state |= flag; + InterlockedOr ((LONG *) &myself->process_state, flag); } - void pop () { myself->process_state &= ~(flag); } + void pop () { InterlockedAnd ((LONG *) &myself->process_state, ~(flag)); } ~push_process_state () { pop (); } }; diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc index 1f26a3ccd..c69213f9a 100644 --- a/winsup/cygwin/pinfo.cc +++ b/winsup/cygwin/pinfo.cc @@ -69,7 +69,7 @@ pinfo::thisproc (HANDLE h) child_info_spawn::handle_spawn. */ init (cygheap->pid, flags, h); - procinfo->process_state |= PID_IN_USE; + InterlockedOr ((LONG *) &procinfo->process_state, PID_IN_USE); procinfo->dwProcessId = myself_initial.dwProcessId; procinfo->sendsig = myself_initial.sendsig; wcscpy (procinfo->progname, myself_initial.progname); @@ -89,7 +89,7 @@ pinfo_init (char **envp, int envc) { environ_init (envp, envc); /* spawn has already set up a pid structure for us so we'll use that */ - myself->process_state |= PID_CYGPARENT; + InterlockedOr ((LONG *) &myself->process_state, PID_CYGPARENT); } else { @@ -108,10 +108,11 @@ pinfo_init (char **envp, int envc) debug_printf ("Set nice to %d", myself->nice); } - myself->process_state |= PID_ACTIVE; - myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED); + InterlockedOr ((LONG *) &myself->process_state, PID_ACTIVE); + InterlockedAnd ((LONG *) &myself->process_state, + ~(PID_INITIALIZING | PID_EXITED | PID_REAPED)); if (being_debugged ()) - myself->process_state |= PID_DEBUGGED; + InterlockedOr ((LONG *) &myself->process_state, PID_DEBUGGED); myself.preserve (); debug_printf ("pid %d, pgid %d, process_state %y", myself->pid, myself->pgid, myself->process_state); diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc index 0bd64963f..f8ba67e75 100644 --- a/winsup/cygwin/signal.cc +++ b/winsup/cygwin/signal.cc @@ -456,9 +456,11 @@ sigaction_worker (int sig, const struct sigaction *newact, sig_clear (sig, true); if (sig == SIGCHLD) { - myself->process_state &= ~PID_NOCLDSTOP; + InterlockedAnd ((LONG *)&myself->process_state, + ~PID_NOCLDSTOP); if (gs.sa_flags & SA_NOCLDSTOP) - myself->process_state |= PID_NOCLDSTOP; + InterlockedOr ((LONG *) &myself->process_state, + PID_NOCLDSTOP); } } diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index 1ffe00a94..8739f18f5 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -252,7 +252,7 @@ proc_subproc (DWORD what, uintptr_t val) vchild->sid = myself->sid; vchild->ctty = myself->ctty; vchild->cygstarted = true; - vchild->process_state |= PID_INITIALIZING; + InterlockedOr ((LONG *)&vchild->process_state, PID_INITIALIZING); vchild->ppid = myself->pid; /* always set last */ } break; diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index 8016f0864..06b84236d 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -543,7 +543,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, refresh_cygheap (); if (c_flags & CREATE_NEW_PROCESS_GROUP) - myself->process_state |= PID_NEW_PG; + InterlockedOr ((LONG *) &myself->process_state, PID_NEW_PG); if (mode == _P_DETACH) /* all set */; @@ -603,7 +603,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, ::cygheap->user.deimpersonate (); if (!real_path.iscygexec () && mode == _P_OVERLAY) - myself->process_state |= PID_NOTCYGWIN; + InterlockedOr ((LONG *) &myself->process_state, PID_NOTCYGWIN); cygpid = (mode != _P_OVERLAY) ? create_cygwin_pid () : myself->pid; @@ -705,7 +705,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, myself->sendsig = myself->exec_sendsig; myself->exec_sendsig = NULL; } - myself->process_state &= ~PID_NOTCYGWIN; + InterlockedAnd ((LONG *) &myself->process_state, ~PID_NOTCYGWIN); /* Reset handle inheritance to default when the execution of a' non-Cygwin process fails. Only need to do this for _P_OVERLAY since the handle will be closed otherwise. Don't need to do -- 2.45.1
