Re: Is the following code MT-Safe?
From: Doug Harrison [MVP] (dsh_at_mvps.org)
Date: 07/01/04
- Next message: Scott McPhillips [MVP]: "Re: VCTERM Sample Program"
- Previous message: David Lowndes: "Re: Distribution of MFC42.dll"
- In reply to: Ken Durden: "Is the following code MT-Safe?"
- Next in thread: Ken Durden: "Re: Is the following code MT-Safe?"
- Reply: Ken Durden: "Re: Is the following code MT-Safe?"
- Messages sorted by: [ date ] [ thread ]
Date: Wed, 30 Jun 2004 19:43:09 -0500
Ken Durden wrote:
>Explanation follows code example, MFC synchronization objects used
>because thats what I know how to use. Some psuedo-code may be present
>where coding in all the parameters wasn't meaningful.
>
>class A
>{
>public:
> A() :
> _bRunning(false),
> _stopEvent(FALSE,FALSE),
> _stoppedEvent(FALSE,FALSE)
> {}
>
> void start()
> {
> CSingleLock( &crit, TRUE );
>
> AfxBeginThread(ThreadFunc, (LPVOID) this, ... );
> }
What do you hope to accomplish with this? First, you're not protecting any
data from concurrent access that I can see. Second, your CSingleLock
statement is completely wrong. It constructs a temporary object that is
immediately destroyed, so it serves no purpose whatsoever. You need to
declare a lock variable, e.g.
CSingleLock lock( &crit, TRUE );
This variable will be destroyed and the mutex[*] released when the scope
which contains it is exited, such that the code that is executed between the
declaration of "lock" and its destruction upon exit of its scope really is a
critical section WRT "crit".
[*] Although "crit" is a CCriticalSection and thus a Windows
CRITICAL_SECTION under the hood, I'm calling it a "mutex", because it's
easier to talk about that way. Windows has CRITICAL_SECTION and HANDLE-based
mutexes, and if important I'll indicate which I mean. I really wish they had
called CRITICAL_SECTION "LIGHTWEIGHT_MUTEX" or something, rather than
co-opting the "critical section" term to describe an object, when it's
traditionally been used to describe a region of code that should be
protected by a mutex.
> void stop()
> {
> CSingleLock( &crit, TRUE );
>
> _stoppedEvent.Reset();
> _stopEvent.Set();
>
> ::WaitForSingleObject( _stoppedEvent, INFINITE );
> }
Again, there's a pointless CSingleLock.
> void test()
> {
> CSingleLock( &crit, TRUE );
>
> if( _bRunning )
> stop();
>
> assert( !_bRunning );
> }
Assuming you fix the CSingleLock statement as previously described, it can
serve to synchronize access to _bRunning, provided all MT access to
_bRunning is so protected. But you don't protect it in ThreadFunc below, so
it serves no purpose here.
>private:
> static UINT ThreadFunc( LPVOID pParam )
> {
> A * a = (A*) pParam;
>
> a->_bRunning = true;
> ::WaitForSingleObject( a->_stopEvent, INFINITE );
> a->_bRunning = false;
>
> a->_stoppedEvent.Set();
> }
You can probably get rid of _stoppedEvent and simply wait on the thread
handle. See this message for more on this:
http://groups.google.com/groups?selm=m3ep501falcm3ecgdb1tsof6p42f32fvk7%404ax.com
> CCriticalSection crit;
> CEvent _stopEvent;
> CEvent _stoppedEvent;
> bool _bRunning;
>};
>
>int main()
>{
> A a;
> a.start();
> a.test();
>}
>
>In this simplistic example, assume the only two threads you see are
>the only two which are relevant. In this simple example, am I
>guaranteed that the assert in the test function will not fire?
>
>I'm worried that the main thread will read the value of _bRunning in
>the test() function, and then when it comes time to do the assert,
>simply re-use the existing value it has already read, rather than
>going back to main memory to get the new value which was set by the
>other thread. Is this a valid concern?
>
>Does _bRunning need to be tagged as volatile?
>
>My understanding from school and reading is that it does, but that
>supposition blows so many holes in the way most MT programs I've seen
>are written its scary.
If all access to _bRunning is protected by a mutex, then volatile serves no
purpose other than to slow down your code and possibly force you to cast
volatile away, the latter a major concern when you're talking about objects
of class type. If you choose not to synchronize access to the variable, you
can sometimes get away with merely declaring it volatile, depending on your
machine architecture and the semantics your compiler vendor chooses to
attach to volatile. See the end of this message for links to messages which
go into more detail on this.
>For example, here's another example.. even simpler:
>
>class B
>{
> public:
> B() : m_pList(NULL) {}
>
> void add( int n )
> {
> CSingleLock( &crit, TRUE );
>
> if( m_pList == NULL )
> m_pList = new std::list<int>();
>
> m_pList->push_back( n );
> }
>
> int size()
> {
> CSingleLock( &crit, TRUE );
>
> if( m_pList == NULL )
> return 0;
>
> return m_pList->size();
> }
>
> private:
> CCriticalSection crit;
> std::list<int> * m_pList;
>};
>
>In this example, does m_pList need to be declared as
>std::list<int> * volatile m_pList
Fix your CSingleLocks as described earlier, and the answer is "no", because
all MT access to m_pList is protected by the same mutex.
>Suppose I have a dozen threads all competing to add items to this
>list, without volatile, is there a possibility that one thread will
>have a "cached" value of m_pList which is out of date?
Fix your CSingleLocks as described earlier, and the answer is "no", because
all MT access to m_pList is protected by the same mutex.
>If the threads
>only called add(), it seems that there is no way for the NULL value of
>m_pList to be cached in a register; what if the threads also called
>size() prior to calling add, however, would this introduce a
>possibility for the pointer value to be stored in a register?
Fix your CSingleLocks as described earlier, and the answer is "no", because
all MT access to m_pList is protected by the same mutex.
>Given the relatively small set of registers on Intel processors, how
>big an issue is volatile on that platform?
One has nothing to do with the other.
>What effect does locality
>of referencing have? If I reference the same variable 30 C lines apart
>from each other does that make a difference from accessing it 2 lines
>apart?
In SMP systems more aggressive than x86, there are memory visibility and
ordering issues such that CPU X may not observe the same values in memory as
CPU Y. Such CPUs have "memory barrier" instructions, which enforce a
consistent view of memory. They're included in the synchronization
primitives such as mutex lock/unlock operations, but may not be implicit in
the access of volatile variables. Whether or not volatile implies memory
barrier depends on the whims of the compiler writer.
>Do non-inlined function-calls change this behavior?
The semantics of a function call don't vary with inlining.
>What are the general guidelines for using volatile? Even if I've got
>my locks and events all lined up perfectly, it seems I can still get
>burnt without it. I've got a beefed up scenario which models class A
>(the first one), I'm going to add volatile to see if I can change the
>behavior, but its (dread) low-frequency intermittent problem, so I
>won't know for a month if it helped probably.
The keyword volatile is neither necessary nor sufficient for most MT
programming. See these messages for more on when volatile is useful:
http://groups.google.com/groups?selm=7hl7205u4ksejokc9ormufeo02pb9i1stu%404ax.com
http://groups.google.com/groups?selm=k7fa209epgjcn2rep0rqks2ee0f4jcitgu%404ax.com
Here's a link to a message in this group which debunks a repeated claim that
volatile is always necessary in MT programming. For the full picture, be
sure to read the messages and web pages it links to:
http://groups.google.com/groups?selm=h2evrvkjb2a0r69pp4j0280ibegttvvm9u%404ax.com
-- Doug Harrison Microsoft MVP - Visual C++
- Next message: Scott McPhillips [MVP]: "Re: VCTERM Sample Program"
- Previous message: David Lowndes: "Re: Distribution of MFC42.dll"
- In reply to: Ken Durden: "Is the following code MT-Safe?"
- Next in thread: Ken Durden: "Re: Is the following code MT-Safe?"
- Reply: Ken Durden: "Re: Is the following code MT-Safe?"
- Messages sorted by: [ date ] [ thread ]
Relevant Pages
|
