On 03/03/2025 18:53, Corinna Vinschen wrote:
On Mar 3 23:39, Takashi Yano wrote:
On Mon, 3 Mar 2025 14:01:08 +0100
Corinna Vinschen wrote:
but now that I'm writing it I'm even more unsure this is necessary.
The only two places doing an And and an Or are doing it with the
exact same flags. And the combination of PID_ACTIVE and the other
three flags is never tested together.
What do you think?
No other code touches these flags except for:
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;
Moreover, using InterlockedOr()/InterlockedAnd() can ensure that
the other flags than the current code is modifying will be kept
during modification. So using InterlockedCompareExchange() might
be over the top.
Okidoki!
Either way, I would add a volatile to _pinfo::process_state.
Thanks. I will.
Great. Feel free to push the patch without sending another patch
submission to cygwin-patches.
I think Takashi-san must be about due another gold star, as he's been
doing some sterling work recently, fixing complex and difficult to
reproduce bugs!
