Discussion:
Efficient way to synchronize bool variables
(too old to reply)
Faisal
2009-11-11 12:09:41 UTC
Permalink
In my program there are boolean vaiables which are shared between a
number of threads.

In the below code m_bRunning and m_bStopped are shared variables. The
function Execute() can be called from any of the threads.I want to
synchronize the accessing of the variable.


void OnStart()
{
m_bStopped = false;
m_bRunning = true;
}

void OnStop()
{
m_bRunning = false;
}

void Execute()
{
if( m_bRunning )
{
//Do some tasks
}
else
{
if( !m_bStopped )
{
//Do some tasks
m_bStopped = true;
}
}
}


Does simply declaring the variables as volatile would solve my
problem?
Or do i need interlockedXXX functions here.

What is the right way to solve this type of problems?
Igor Tandetnik
2009-11-11 13:20:38 UTC
Permalink
Post by Faisal
In my program there are boolean vaiables which are shared between a
number of threads.
In the below code m_bRunning and m_bStopped are shared variables. The
function Execute() can be called from any of the threads.I want to
synchronize the accessing of the variable.
void OnStart()
{
m_bStopped = false;
m_bRunning = true;
}
void OnStop()
{
m_bRunning = false;
}
void Execute()
{
if( m_bRunning )
{
// RunningTask()
}
else
{
if( !m_bStopped )
{
// FinalizeTask()
m_bStopped = true;
}
}
}
Does simply declaring the variables as volatile would solve my
problem?
Or do i need interlockedXXX functions here.
I suspect neither will help, but it depends on precisely what "your problem" is that you are trying to solve. For example, what should happen if m_bRunning turns false while RunningTask is executing? Is it OK to run RunningTask in parallel with FinalizeTask? Is it OK for FinalizeTask to be run more than once by multiple threads (many threads can enter Execute at once and see m_bRunning==m_bStopped==false)?
Post by Faisal
What is the right way to solve this type of problems?
I'd start with _formulating_ it.
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Goran
2009-11-11 13:24:21 UTC
Permalink
Post by Faisal
In my program there are boolean vaiables which are shared between a
number of threads.
In the below code m_bRunning and m_bStopped are shared variables. The
function Execute() can be called from any of the threads.I want to
synchronize the accessing of the variable.
void OnStart()
{
        m_bStopped = false;
        m_bRunning = true;
}
void OnStop()
{
        m_bRunning = false;
}
void Execute()
{
        if( m_bRunning )
        {
                //Do some tasks
        }
        else
        {
                if( !m_bStopped )
                {
                        //Do some tasks
                        m_bStopped = true;
                }
        }
}
Does simply declaring the variables as volatile would solve my
problem?
Or do i need interlockedXXX functions here.
Volatile is all but useless on any hardware that has any form of CPU
cache.

InterlockedXXX are OK. I use a class similar to (warning: compiled and
tested with head-compiler):

class CThreadSafeFlag
{
mutable LONG m_Flag;
public:
CThreadSafeFlag(bool bSet=false) : m_Flag(bSet) {}
bool IsSet() const { return InterlockedCompareExchange(&m_Flag, 2,
2) == true; }
void Set() { InterlockedExchange(&m_Flag, 1); }
void Reset() { InterlockedExchange(&m_Flag, 0); }
};

IsSet simply uses "impossible" exchange value to avoid changing
m_Flag, but still return it's value in a thread-safe manner.

Of course, this is all completely orthogonal to any race conditions I
might produce in rest of the code.

Goran.
Igor Tandetnik
2009-11-11 13:52:00 UTC
Permalink
Post by Goran
class CThreadSafeFlag
{
mutable LONG m_Flag;
CThreadSafeFlag(bool bSet=false) : m_Flag(bSet) {}
bool IsSet() const { return InterlockedCompareExchange(&m_Flag, 2,
2) == true; }
void Set() { InterlockedExchange(&m_Flag, 1); }
void Reset() { InterlockedExchange(&m_Flag, 0); }
};
IsSet simply uses "impossible" exchange value to avoid changing
m_Flag, but still return it's value in a thread-safe manner.
InterlockedCompareExchange(&m_Flag, 0, 0) would have worked just as well. It says "set the variable to zero only if it's zero already", so it's always a no-op.
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Faisal
2009-11-11 14:37:32 UTC
Permalink
Post by Igor Tandetnik
Post by Goran
class CThreadSafeFlag
{
  mutable LONG m_Flag;
  CThreadSafeFlag(bool bSet=false) : m_Flag(bSet) {}
  bool IsSet() const { return InterlockedCompareExchange(&m_Flag, 2,
2) == true; }
  void Set() { InterlockedExchange(&m_Flag, 1); }
  void Reset() { InterlockedExchange(&m_Flag, 0); }
};
IsSet simply uses "impossible" exchange value to avoid changing
m_Flag, but still return it's value in a thread-safe manner.
InterlockedCompareExchange(&m_Flag, 0, 0) would have worked just as well. It says "set the variable to zero only if it's zero already", so it's always a no-op.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,
Actually, I misrepresented the problem here. The Execute() function
are called only by one thread.
OnStart() and OnStop() functions are called by UI thread. I have to
synchronize between these two threads.

In this case which method would you prefer.

If volatile does the synchronization, why we need the InterLockedXXX
functions?
Also, in which cases volatile is useful
Goran
2009-11-11 15:51:14 UTC
Permalink
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Post by Faisal
Also, in which cases volatile is useful
Only thing volatile does is to cause the compiler not to optimize
access to a variable V, that has it's place in main memory.

This is because compilers can note that V is accessed frequently, and
decide it would be useful to put it e.g. in a CPU register, at least
for some parts of code X. If compiler does that, and if code part X
relies on code part Y to modify V __concurrently__, code part X might
err.

So, volatile works in following cases (I can't think of more):

1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system

None of these 3 situations are likely to be yours, so, forget
volatile.

Goran.
Igor Tandetnik
2009-11-11 17:12:55 UTC
Permalink
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See

http://msdn.microsoft.com/en-us/library/12a04hfd.aspx

the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
Igor Tandetnik

With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Goran
2009-11-12 08:57:38 UTC
Permalink
Post by Igor Tandetnik
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
Ah, OK. Thanks for putting me right there (nice catch about using 0
instead of e.g. 2, too). I think that Itanium has weaker coherence,
though.

I still think (that's actually directed to Joe) that it is better to
just use appropriate system-provided atomic operations (here:
InterlockedXXX) over volatile, because of:

* general habits: relevant piece I found in C99 (I didn't check, but I
doubt it's significantly different for C++) is: "An object that has
volatile-qualified type may be modified in ways unknown to the
implementation or have other unknown side effects." I read that as
e.g. "external force changes memory (e.g. a peripheral mapped into
system's memory space, point 1 up there)". Now, sure, language knows
not of threading, but implementation does, so then it seems slightly
misguided to use volatile as defined, but in the context of threading.
Also, when on Linux some people tend to come on you with an axe if you
use "volatile" ;-).

* compiler change: OK, MS compiler does it "right", but what are
others doing?

* hardware change: who knows what will performance freaks at Intel/AMD
come up with in few years time (although they too they have to worry
about not breaking existing code).

I don't know when performance hit in forcing atomic operations (which
are, as you guys explain, unnecessary on x86/64) will be relevant, but
I gather that will be seldom (and almost surely isn't in the kind of
situation we discuss here).

Goran.
Faisal
2009-11-12 09:34:41 UTC
Permalink
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,

Does this would work.

void OnStart()
{
InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}

void OnStop()
{
InterlockedExchange( (volatile long*)&m_bRunning, FALSE );

}

void Execute()
{

if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
{
//Do some tasks
}
else
{
if( InterlockedCompareExchange ((volatile long*)&m_bStopped, TRUE,
FALSE) )
{
//Do some tasks
}
}

}
Joseph M. Newcomer
2009-11-12 22:43:42 UTC
Permalink
Post by Faisal
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,
Does this would work.
void OnStart()
{
InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}
****
void OnStart()
{
m_bStopped = FALSE;
m_bRunning = TRUE;
}

works because the thread is not stopped and the thread is running. And in the case of
scripting, script state HAS to be thread-local and therefore requires no synchronization!
Otherwise, you cannot have two threads executing the same script, which must be
asynchronous in each thread.
****
Post by Faisal
void OnStop()
{
InterlockedExchange( (volatile long*)&m_bRunning, FALSE );
}
*****
Since the script state has to be per-thread, this member variable has to be a member of
the CWinThread object, and therefore requires no synchronization because there is no
concurrent access.

You always have to examine the complete context; naive solutions that either (a) ignore
synchronization or (b) overuse synchronization when it is not necessary are both wrong.

You have to ask "Who has access to this variable?" "Is it monotonically mutable?" "Is it
thread-local?"

Note that the InterlockedExchange does NOTHING that the assignment statement will not do,
because you don't look at the old value of the variable!
*****
Post by Faisal
void Execute()
{
if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
****
I don't understand why this matters. Either a script is running or not, which is
thread-local state. It would make no sense to make it, say, a global variable; I don't
understand why you are using a global function here, since this is running in the context
of a thread.
*****
Post by Faisal
{
//Do some tasks
}
else
{
if( InterlockedCompareExchange ((volatile long*)&m_bStopped, TRUE,
FALSE) )
{
//Do some tasks
}
}
****
We have found in general that people trying to implement synchronzation "from scratch"
usually get it wrong. Since I cannot understand why you think it has to be atomic, since
it is set only by the thread, I don't see any synchronization requirement at all.

I notice that you are using names like m_bStopped but have not told us what class this is
declared in, but you certainly cannot access a class variable from a global function
called "Execute". Please provide ALL necessary information required to formulate an
answer.
joe
****
Post by Faisal
}
Joseph M. Newcomer [MVP]
email: ***@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
Faisal
2009-11-13 12:56:12 UTC
Permalink
Post by Faisal
Post by Faisal
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,
Does this would work.
void OnStart()
{
   InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
   InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}
****
void OnStart()
    {
        m_bStopped = FALSE;
        m_bRunning = TRUE;
    }
works because the thread is not stopped and the thread is running.  And in the case of
scripting, script state HAS to be thread-local and therefore requires no synchronization!
Otherwise, you cannot have two threads executing the same script, which must be
asynchronous in each thread.
****>void OnStop()
Post by Faisal
{
   InterlockedExchange( (volatile long*)&m_bRunning, FALSE );
}
*****
Since the script state has to be per-thread, this member variable has to be a member of
the CWinThread object, and therefore requires no synchronization because there is no
concurrent access.
You always have to examine the complete context; naive solutions that either (a) ignore
synchronization or (b) overuse synchronization when it is not necessary are both wrong.
You have to ask "Who has access to this variable?"  "Is it monotonically mutable?"  "Is it
thread-local?"
Note that the InterlockedExchange does NOTHING that the assignment statement will not do,
because you don't look at the old value of the variable!
*****
Post by Faisal
void Execute()
{
       if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
****
I don't understand why this matters.  Either a script is running or not, which is
thread-local state.  It would make no sense to make it, say, a global variable; I don't
understand why you are using a global function here, since this is running in the context
of a thread.
*****>        {
Post by Faisal
            //Do some tasks
       }
       else
       {
                    if( InterlockedCompareExchange ((volatile long*)&m_bStopped, TRUE,
FALSE) )
            {
                //Do some tasks
            }
       }
****
We have found in general that people trying to implement synchronzation "from scratch"
usually get it wrong.  Since I cannot understand why you think it has to be atomic, since
it is set only by the thread, I don't see any synchronization requirement at all.  
I notice that you are using names like m_bStopped but have not told us what class this is
declared in, but you certainly cannot access a class variable from a global function
called "Execute".  Please provide ALL necessary information required to formulate an
answer.
                                joe
****
Post by Faisal
}
Joseph M. Newcomer [MVP]
Web:http://www.flounder.com
MVP Tips:http://www.flounder.com/mvp_tips.htm
Hi Joe,

More details below

//from SignalProcessor,h
class CSignalProcessor
{

public:
void OnScriptingStart();
void OnScriptingStop();
void Execute( long* plData, unisgned int unDataSize );

private:
BOOL m_bScriptStopped;
BOOL m_bScriptRunning;
}


This is called by UI thread when user clicks a menu item

void CSignalProcessor::OnScriptingStart()
{
InterlockedExchange( (volatile long*)&m_bScriptStopped, FALSE );
InterlockedExchange( (volatile long*)&m_bScriptRunning, TRUE );
}

This is called by UI thread when user clicks a menu item

void CSignalProcessor::OnScriptingStop()
{
InterlockedExchange( (volatile long*)&m_bScriptRunning, FALSE );
}


This function is called by the data dispatcher thread. This data
dispatcher thread maintains a queue and sempahore. When the data
acquistion thread reads a data( say frame ) from hardware it is pushed
into the queue and the semaphore is released. Data dispatcher thread
WFMO to signal this semaphore. When this thread is woke-up it dequeues
a frame from queue and the function Excute() of CSignalProcessor is
called.

void CSignalProcessor::Execute( long* plData, unsigned int
unDataSize )
{

//Post a message to mainwindow so that the data will be plotted in a
graph window
m_pMainWnd->PostMessage( UWM_LINEDATAUPDATE, (WPARAM)plData, (LPARAM)
unDataSize );

if( InterlockedCompareExchange ((volatile long*)
&m_bScriptRunning, TRUE, TRUE) )
{
//Scripting is enabled.
//Do some processing on the data and keep it
}
else
{
if( InterlockedCompareExchange ((volatile long*)
&m_bScriptStopped, TRUE,FALSE) )
{
//Scripting stopped
//Combine the data and save in to a file
}
}
}


In this context, do i need the InterlockedXXX operations?

Faisal
2009-11-12 09:46:32 UTC
Permalink
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,

Does this solve the problem?

void OnStart()
{
InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
InterlockedExchange( (volatile long*)&m_bRunning, TRUE );

}

void OnStop()
{
InterlockedExchange( (volatile long*)&m_bRunning, FALSE );

}

void Execute()
{

if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
{
//Do some tasks
}
else
{
if( InterlockedCompareExchange ((volatile long*)
&m_bStopped, TRUE,FALSE) )
{
//Do some tasks
}
}

}
Goran
2009-11-12 11:29:45 UTC
Permalink
Post by Faisal
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
Igor,
Does this solve the problem?
void OnStart()
{
        InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
        InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}
void OnStop()
{
        InterlockedExchange( (volatile long*)&m_bRunning, FALSE );
}
void Execute()
{
        if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
        {
             //Do some tasks
        }
        else
        {
             if( InterlockedCompareExchange ((volatile long*)
&m_bStopped, TRUE,FALSE) )
             {
                 //Do some tasks
             }
        }
}
Actually, as Igor explained, on x86/64, even volatile is fine.

That said, these casts aren't nice! That's dangerous, it's only OK as
long as your flags are of the same size as LONG (also, they need to be
correctly aligned on x86/64, I think, but if you don't play with
#pragma pack, that's going to be OK). Just use LONG there. (I
personally don't matter one bit if names stay prefixed with "b_").

Also, with InterlockedXXX, volatile does not matter in the slightest.

Provided that my little class above works, you get:

void OnStart()
{
m_bStopped.Reset();
m_bRunning.Set();
}

void OnStop()
{
m_bRunning.Reset();
}

void Execute()
{

if( m_bRunning.IsSet())
{
//Do some tasks
}
else
{
if( !m_bStopped.IsSet())
{
//Do some tasks
}
}
}

Note also: with your InterlockedXXX calls there, you kinda mix lower-
level giberrish with higher-level logic. That's harder to read, so
it's slightly better to abstract that away.

Goran.
Joseph M. Newcomer
2009-11-12 22:47:26 UTC
Permalink
See below,,,
Post by Goran
Post by Faisal
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
Igor,
Does this solve the problem?
void OnStart()
{
        InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
        InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}
void OnStop()
{
        InterlockedExchange( (volatile long*)&m_bRunning, FALSE );
}
void Execute()
{
        if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
        {
             //Do some tasks
        }
        else
        {
             if( InterlockedCompareExchange ((volatile long*)
&m_bStopped, TRUE,FALSE) )
             {
                 //Do some tasks
             }
        }
}
Actually, as Igor explained, on x86/64, even volatile is fine.
****
I do not see anywhere he said that. volatile, even with memory barriers, does NOT create
thread-safe modifications, and will not work. If you can't do it in a single instruction,
it is not thread-safe. A memory barrier instruction is a second instruction. Do NOT
confuse volatility with synchronization. They are at best vaguely related.
*****
Post by Goran
That said, these casts aren't nice! That's dangerous, it's only OK as
long as your flags are of the same size as LONG (also, they need to be
correctly aligned on x86/64, I think, but if you don't play with
#pragma pack, that's going to be OK). Just use LONG there. (I
personally don't matter one bit if names stay prefixed with "b_").
*****
The cast is really bad because the variable should be declared as volatile long, and the
cast should be unnecessary.
****
Post by Goran
Also, with InterlockedXXX, volatile does not matter in the slightest.
*****
Agreed.
*****
Post by Goran
void OnStart()
{
m_bStopped.Reset();
m_bRunning.Set();
}
void OnStop()
{
m_bRunning.Reset();
}
void Execute()
{
if( m_bRunning.IsSet())
{
//Do some tasks
}
else
{
if( !m_bStopped.IsSet())
{
//Do some tasks
}
}
}
Note also: with your InterlockedXXX calls there, you kinda mix lower-
level giberrish with higher-level logic. That's harder to read, so
it's slightly better to abstract that away.
Goran.
Joseph M. Newcomer [MVP]
email: ***@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
Goran
2009-11-13 08:01:07 UTC
Permalink
Post by Goran
Actually, as Igor explained, on x86/64, even volatile is fine.
****
I do not see anywhere he said that.  volatile, even with memory barriers, does NOT create
thread-safe modifications, and will not work.  If you can't do it in a single instruction,
it is not thread-safe.  A memory barrier instruction is a second instruction.  Do NOT
confuse volatility with synchronization.  They are at best vaguely related.
*****
I absolutely agree with you that one should not use volatile for
thread safety, that's OK.

My understanding is that both x86 and 64 provides for strong cache
coherence (and that's what Igor said, too). Couple that with volatile
forcing the compiler to issue memory reads/writes for volatile data
(and avoiding e.g. "register" optimization), and with variable being
"atomic" on x86/64, and you get correct behavior in this case. (But
that's what you are saying all along, too).

Goran.
Malachy Moses
2009-11-12 20:16:34 UTC
Permalink
Post by Faisal
Post by Igor Tandetnik
Post by Goran
Post by Faisal
If volatile does the synchronization, why we need the InterLockedXXX
functions?
Volatile does not do any synchronization, you are mistaken.
Actually, as of VC8, the compiler generates memory barrier instructions (on those architectures that need them) when accessing volatile variables. See
http://msdn.microsoft.com/en-us/library/12a04hfd.aspx
the part that talks about acquire and release semantics. This is MS-specific and non-portable.
Post by Goran
1. hardware that has no CPU cache and code relies on some peripheral
equipment to change main memory contents
2. concurrent access on a multi-CPU systems with no per-CPU cache
3. concurrent access on a single-CPU system
4. Multi-CPU system that features strong cache coherence - as is the case with all x86 CPUs. Systems with weak cache coherence (the kind where one CPU can write a value to memory but another can observe a stale old value from the cache indefinitely, the kind that provides and requires memory barrier instructions) are actually not all that widespread.
--
With best wishes,
    Igor Tandetnik
With sufficient thrust, pigs fly just fine. However, this is not necessarily a good idea. It is hard to be sure where they are going to land, and it could be dangerous sitting under them as they fly overhead. -- RFC 1925
Igor,
Does this solve the problem?
void OnStart()
{
        InterlockedExchange( (volatile long*)&m_bStopped, FALSE );
        InterlockedExchange( (volatile long*)&m_bRunning, TRUE );
}
void OnStop()
{
        InterlockedExchange( (volatile long*)&m_bRunning, FALSE );
}
void Execute()
{
        if( InterlockedCompareExchange ((volatile long*)&m_bRunning,
TRUE, TRUE) )
        {
             //Do some tasks
        }
        else
        {
             if( InterlockedCompareExchange ((volatile long*)
&m_bStopped, TRUE,FALSE) )
             {
                 //Do some tasks
             }
        }
}
That might not be enough, at not enough for the OnStart() function.

The reason is that although the Interlocked functions are themselves
atomic, the OnStart function in its entirety is not atomic. You thus
might find yourself in a situation where OnStart has changed bStopped,
is interrupted by another thread that does unexpected things because
bStopped and bRunning are in an unexpected combination, and then is
resumed doing yet further unexpected things with bRunning.

If you need to coordinate the state of two different bools, so that
their combination of values is sensible at all times, then I think it
would be difficult to accomplish using the Interlocked functions
alone. You probably need synchronization like a critical section.
Loading...