Re: [ros-diffs] [gedmurphy] 19312: Improve stopping control of the service

[Thomas Weidenmueller]

[e-mail] wrote:
> Improve stopping control of the service
>
> @@ -137,14 +137,14 @@
>  {
>      case SERVICE_CONTROL_SHUTDOWN: /* fall through */
>      case SERVICE_CONTROL_STOP:
>
>        bShutDownFlag = TRUE;
>
>        bShutDown = TRUE;
>
>        UpdateStatus(SERVICE_STOP_PENDING, -1);
>        break;
>    case SERVICE_CONTROL_PAUSE:
>
>        bPauseFlag = TRUE;
>
>        bPause = TRUE;
>
>        break;
>    case SERVICE_CONTROL_CONTINUE:
>
>        bPauseFlag = FALSE;
>
>        bPause = FALSE;

You might want to use InterlockedExchange to make it thread-safe.

- Thomas

[Ged Murphy]

Thomas Weidenmueller wrote:
> [e-mail] wrote:
> > Improve stopping control of the service
> >
> > @@ -137,14 +137,14 @@
> >  {
> >      case SERVICE_CONTROL_SHUTDOWN: /* fall through */
> >      case SERVICE_CONTROL_STOP:
> >
> >        bShutDownFlag = TRUE;
> >
> >        bShutDown = TRUE;
> >
> >        UpdateStatus(SERVICE_STOP_PENDING, -1);
> >        break;
> >    case SERVICE_CONTROL_PAUSE:
> >
> >        bPauseFlag = TRUE;
> >
> >        bPause = TRUE;
> >
> >        break;
> >    case SERVICE_CONTROL_CONTINUE:
> >
> >        bPauseFlag = FALSE;
> >
> >        bPause = FALSE;
>
> You might want to use InterlockedExchange to make it thread-safe.
> Thomas

Hi Thomas.

Could you explain what you mean?
Those variables are only modified within the base thread, all other 
threads just check the value and act upon it.
Why would that not be thread safe?

Thanks,
Ged.

[Thomas Weidenmueller]

Ged Murphy wrote:
> Could you explain what you mean?

As you want to change the value of the variables which are used in a
multithreaded environment you need to used the Interlocked* functions to
ensure an atomic change in all environments (UP, MP, ...), and also to
make sure it's portable.
> Those variables are only modified within the base thread, all other
> threads just check the value and act upon it.
> Why would that not be thread safe?

You're assuming the code will only run on a x86 system. In fact, as long
as the variables are aligned properly it is an atomic operation. But
don't forget the other architectures and possibly upcoming technologies
based on x86 that could void this assumption.

I have to admit that the code wouldn't break, even in circumstances
where the operation wouldn't be atomic, but technically the code still
isn't thread-safe this way.

- Thomas