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

[Corinna Vinschen]

Hi Takashi,

On Mar  1 08:33, Takashi Yano wrote:
> 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.

Is this really sufficent?  I'm asking because of...

> @@ -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)
>      {

...places like these.  Every single Interlocked call is safe in itself,
but what if somebody else changes something between the two interlocked
calls?  Maybe this should be done with InterlockedCompareExchange.

Also, I think it might be better to define process_state as volatile.

Other than that (and I'm not sure if I'm not just being paranoid here),
the patchset looks good.


Thanks,
Corinna