[PATCH v3 2/3] Cygwin: signal: Fix a race issue on modifying _pinfo::process_state

[Corinna Vinschen]

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.


Thanks,
Corinna