On Mar 6 14:55, Jon Turney wrote: > 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!
Absolutely! Yesterday I was even mulling over a pink plush hippo, but Takashi already has one over his fireplace and I wasn't sure if a second one isn't taking too much space. Corinna
