Is this thread safe?

[Sacha]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

with an class hierarchie like this


class CBase
{
public:
virtual void Special() = 0;
void DoOneStep()
{
Special();
};
};

class A: public CBase
{
void Special();
};

class B: public CBase
{
void Special();
};


And a control thread, which is able to swap objects while the working
thread is running:

//global objects and pointer:
A a;
B b;
CBase *g_pObject;


g_pObject = &a;

// and later on...
g_pObject = &b;


I also use a HANDLE, which is a static member of CBase, to synchronize
object member calls from within the working and control thread.

To have an idea about the timimg:

DoOneStep is called about every 10 ms, an object swap (or object member
call) occurs upon user interactio, so lets say every 10s.


Is this safe, if there is a consistent state after every DoOneStep?
What could I do better?


The whole thing is a quick and dirty project, and the object(s) are
already well elaborated. Historically, there was only one object, and I
did not need to swap. Inheritance seems to be the easiest way to proceed
with different objects/classes.

I prefer a quick and not so dirty solution rather than a complete
redesign...


Thanks for any insight

-Sacha

 

[Maxim Yegorushkin]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}
[]

And a control thread, which is able to swap objects while the working
thread is running:

//global objects and pointer:
A a;
B b;
CBase *g_pObject;


g_pObject = &a;

// and later on...
g_pObject = &b;


I also use a HANDLE, which is a static member of CBase, to synchronize
object member calls from within the working and control thread.

To have an idea about the timimg:

DoOneStep is called about every 10 ms,

From your code it seems that the worker thread calls it as many times

as it can without any delay. Does DoOneStep() implement the waiting?

an object swap (or object member
call) occurs upon user interactio, so lets say every 10s.

Is this safe, if there is a consistent state after every DoOneStep?
What could I do better?

Use locking or atomic functions such as
InterlockedCompareExchangePointer to change and read the pointer value.
This ensures coherent pointer variable memory visibility between
threads.

 

[Sacha]

as it can without any delay. Does DoOneStep() implement the waiting?

Yes DoOneStep() implements the (rather complex) timing.

Use locking or atomic functions such as
InterlockedCompareExchangePointer to change and read the pointer value.
This ensures coherent pointer variable memory visibility between
threads.

I already use locking for other parts (see OP). I never heard of
InterlockedCompareExchangePointer before, I just looked it up (yes, my
platform is WinXP). However, I don't see how locking or
InterlockedCompareExchangePointer is properly applied here.

The question is, if it is generally save to change the value of a
pointer to an object, before a member call via this pointer has
returned, or if there is only a potential problem when the modification
occures *exactly* while the call is initialized?

-Sacha

 

[peter.koch.larsen]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

with an class hierarchie like this


class CBase
{
public:
virtual void Special() = 0;
void DoOneStep()
{
Special();
};
};

class A: public CBase
{
void Special();
};

class B: public CBase
{
void Special();
};


And a control thread, which is able to swap objects while the working
thread is running:

//global objects and pointer:
A a;
B b;
CBase *g_pObject;


g_pObject = &a;

// and later on...
g_pObject = &b;


I also use a HANDLE, which is a static member of CBase, to synchronize
object member calls from within the working and control thread.

I do not see the relevance of this point.

To have an idea about the timimg:

DoOneStep is called about every 10 ms, an object swap (or object member
call) occurs upon user interactio, so lets say every 10s.

Timing is also irrelevant here.

Is this safe, if there is a consistent state after every DoOneStep?

Not portably. Imagine a processor where a pointer-swap is not atomic.
If one thread saw a partial swap, garbage would result. (E.g. adress of
a is 0x1111 and adress of b is 0x2222. Thread sees 0x1122).
Another problem is that the workerthread might not see the update
performed by the thread that switches the pointers.

What could I do better?

Portably nothing as C++ does not know about threads. Otherwise use some
kind of mutex or critical section or whatever it is called. There are
also lockfree solutions where you might be required to go closer to the
metal. This solution is probably less portable.

The whole thing is a quick and dirty project, and the object(s) are
already well elaborated. Historically, there was only one object, and I
did not need to swap. Inheritance seems to be the easiest way to proceed
with different objects/classes.

I prefer a quick and not so dirty solution rather than a complete
redesign...


Thanks for any insight

-Sacha

/Peter

 

[Maxim Yegorushkin]

Sacha wrote:

[]

I already use locking for other parts (see OP). I never heard of
InterlockedCompareExchangePointer before, I just looked it up (yes, my
platform is WinXP). However, I don't see how locking or
InterlockedCompareExchangePointer is properly applied here.

The question is, if it is generally save to change the value of a
pointer to an object, before a member call via this pointer has
returned,

It's safe, provided that the called function does not access the
pointer.

or if there is only a potential problem when the modification
occures *exactly* while the call is initialized?

There is a potential problem with memory visibility between threads on
SMP. When you change the shared pointer you have to make sure that the
change is noticed by other processors, which is done by using memory
barriers. On Windoze Interlocked* functions impose a memory barrier,
which is what you need.

 

[Joe Seigh]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

[...]
This is not atomic. Thread-safe pointer access aside, what
happens if g_pObect changes after after the test or
during the execution of DoOneStep? Can the objects
be deallocated? If so, what prevents them from being
deallocated while they're being used.

 

[=?ISO-8859-1?Q?Sacha_Sch=E4r?=]

I also use a HANDLE, which is a static member of CBase, to synchronize

I do not see the relevance of this point.


Timing is also irrelevant here.

I agree that this information is not directly relevant for the actual
problem. However, when it comes to the choice of one of different
solutions, it might be important to know what have already been done,
and what the realtime conditions are.

 

[=?ISO-8859-1?Q?Sacha_Sch=E4r?=]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

[...]
This is not atomic. Thread-safe pointer access aside, what
happens if g_pObect changes after after the test or
during the execution of DoOneStep? Can the objects
be deallocated? If so, what prevents them from being
deallocated while they're being used.

The pointer is always pointing to a static object or, in the beginning,
NULL, so no objects are ever deallocated.

The pointer swap is most probly happening during the execution of
DoOneStep, but this pointer is never accessed inside the object.

If I turn the above test and call into one atomic unit, would it be safe
then?

-Sacha

 

[Gianni Mariani]

I already use locking for other parts (see OP). I never heard of
InterlockedCompareExchangePointer before, I just looked it up (yes, my
platform is WinXP). However, I don't see how locking or
InterlockedCompareExchangePointer is properly applied here.

The question is, if it is generally save to change the value of a
pointer to an object, before a member call via this pointer has
returned, or if there is only a potential problem when the modification
occures *exactly* while the call is initialized?

No need for InterlockedCompareExchangePointer here in any cases as long
as the lifetime of the objects being pointed to is longer than any of
the threads that are accessing it and that the object is thread-safe.

 

[Gianni Mariani]

I do not see the relevance of this point.



Timing is also irrelevant here.




Not portably. Imagine a processor where a pointer-swap is not atomic.

It's not actually a swap. It's an assignment.

If one thread saw a partial swap, garbage would result. (E.g. adress of
a is 0x1111 and adress of b is 0x2222. Thread sees 0x1122).

No modern processor does this (if properly aligned). Given, it is
implementation defined, however there is no practical alternative.

Another problem is that the workerthread might not see the update
performed by the thread that switches the pointers.



Portably nothing as C++ does not know about threads. Otherwise use some
kind of mutex or critical section or whatever it is called. There are
also lockfree solutions where you might be required to go closer to the
metal. This solution is probably less portable.

Exactly, so if it works on the platform in question, there is no more
that you can expect in terms of portability. (which makes this whole
thread ot).

 

[Joe Seigh]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

[...]
This is not atomic. Thread-safe pointer access aside, what
happens if g_pObect changes after after the test or
during the execution of DoOneStep? Can the objects
be deallocated? If so, what prevents them from being
deallocated while they're being used.

The pointer is always pointing to a static object or, in the beginning,
NULL, so no objects are ever deallocated.

The pointer swap is most probly happening during the execution of
DoOneStep, but this pointer is never accessed inside the object.

If I turn the above test and call into one atomic unit, would it be safe
then?

Something like

temp = g_pObject; // atomic, temp is local
if (temp) temp->DoOneStep();


Yes, if the objects are initialized before any threads are started to
ensure proper object visibility.

 

[Phil Staite]

Consider I have a working thread like this,

while (1)
{
if ( g_pObject ) g_pObject->DoOneStep();
}

Note that if g_pObject is not declared volatile, there's no guarantee
the worker thread would ever see the switch.

 

[Maxim Yegorushkin]

Phil Staite wrote:

[]

Note that if g_pObject is not declared volatile, there's no guarantee
the worker thread would ever see the switch.

Don't start it. Volatile has nothing to do with multithreading and
visibility.

 

[Marc Mutz]

Maxim Yegorushkin wrote:

Don't start it. Volatile has nothing to do with
multithreading and visibility.

Ummm...

extern bool stop;

// thread 1
int i = 0;
while ( !stop ) {
cout << i++ << endl;
}
exit_thread();

// thread 2
// ...
stop = true;
wait_for( thread1 );

Unless 'stop' is declared volatile, the compiler might
rightfully deduce that the code labeled 'thread 1' can't
change the 'stop' variable's contents, and thus move the
variable lookup outside of the loop, makeing the loop
effectivly
if ( !stop )
while ( true )
cout << i++ << endl;
Thus, 'thread 2' blocks infinitely (or just much longer
than needed) in the wait_for() call.

How come you claim that volatile has nothing to do with
multithreading if the usual guideline is to make all data
shared between threads volatile, and even use volatile on
class types to provide a sub-interface that's safe
calling for shared instances?

Marc

 

[Maxim Yegorushkin]

Maxim Yegorushkin wrote:


Ummm...

extern bool stop;

// thread 1
int i = 0;
while ( !stop ) {
cout << i++ << endl;
}
exit_thread();

// thread 2
// ...
stop = true;
wait_for( thread1 );

Unless 'stop' is declared volatile, the compiler might
rightfully deduce that the code labeled 'thread 1' can't
change the 'stop' variable's contents, and thus move the
variable lookup outside of the loop, makeing the loop
effectivly
if ( !stop )
while ( true )
cout << i++ << endl;
Thus, 'thread 2' blocks infinitely (or just much longer
than needed) in the wait_for() call.

This won't happen. There are IO calls after which a compiler can not
assume state of any variables and must reload them from memory.

How come you claim that volatile has nothing to do with
multithreading if the usual guideline is to make all data

There is no such guideline.

shared between threads volatile, and even use volatile on
class types to provide a sub-interface that's safe
calling for shared instances?

Volatile can only prevent compiler reordering and has no effect on
processor ordering. The proper guideline is to use memory barriers. The
subject has already been beaten to death in newsgroups, google groups
for "volatile (multithreading | visibility)".

For MSVC all Interlocked* and synchronization primitive calls impose
memory barriers. For POSIX threads at least locking/unlocking a mutex,
waiting on a condition variable impose a memory barrier. Volatile is
neither sufficient nor necessary.

I'm not going to elaborate any further, wealth of information is
available in the groups.

 

[Joe Seigh]

How come you claim that volatile has nothing to do with
multithreading if the usual guideline is to make all data
shared between threads volatile, and even use volatile on
class types to provide a sub-interface that's safe
calling for shared instances?

Volatile isn't defined with respect to threading. Also
it's implmentation dependent. So it's not a very useful
thing to present as part of a threading api since there's
no way to know what it means.

As part of implementing a synchronization primative, volatile
might be useful in some situtions as an compiler optimization
directive provided the implementer can determine the compiler's
implementation of volatile. There are other tricks as well.

So, for example, the sychronization primative here would likely be
atomic<*T> which has the right atomicity and acquire semantics.
atomic<*T> would look a lot like the revised JSR-133 Java volatile which
except for spelling isn't the same as C/C++ volatile. C/C++
volatile may or may not be used in the implementation but it
it was it would be internal and not exposed at the api level.

Anyway, unless you were literally born yesterday, you should know
volatile has been discussed to death and the opinion of the experts
at least is that it's useless as a threading api.

 

[Alexander Terekhov]

Joe Seigh wrote:
[...]

So, for example, the sychronization primative here would likely be
atomic<*T> which has the right atomicity and acquire semantics.

Naked-competing for both swap() in control thread and load() in his
working thread. As long as OP merely swaps in another thread (not
performing any accesses to *T at all in his control thread), there's
no need for acquire (and/or release) semantics.

atomic<*T> would look a lot like the revised JSR-133 Java volatile which
except for spelling isn't the same as C/C++ volatile.

Revised Java volatiles are fully dressed (provide RCsc semantics).
Striptease is not for (Java) kids. C++ atomic<> (with naked stuff)
is for adults.

regards,
alexander.

 

[Joe Seigh]

Revised Java volatiles are fully dressed (provide RCsc semantics).
Striptease is not for (Java) kids. C++ atomic<> (with naked stuff)
is for adults.

If you design an api with extremely subtle but important differences,
then yes, that would be a problem. You should design an api with the
target audience in mind.

 

[Marc Mutz]

Joe Seigh wrote:

Anyway, unless you were literally born yesterday, you
should know volatile has been discussed to death and the
opinion of the experts at least is that it's useless as
a threading api.

<snip>

Seems I was.
Or maybe I wasn't on ng's long enough. I don't remember
seeing this discussed in any of the standard books, nor
in a CUJ article. I do remember, however, a CUJ article
from Andrei about using the volatile qualifier for
creating sub-interfaces of class types (const-like). But
that was in 2001 or so. Seems it's been outdated since.

Marc

 

[Leon Mergen]

How come you claim that volatile has nothing to do with

multithreading if the usual guideline is to make all data
shared between threads volatile, and even use volatile on
class types to provide a sub-interface that's safe
calling for shared instances?

Does this also apply to data shared between threads, protected by some
mutex ?

I have never learned that had to be done, but I do declare my data
volatile if it isn't protected.

Regards,

Leon Mergen