Re: Boost.Threads - Open Questions from the Review

From: William Kempf (williamkempf_at_[hidden])
Date: 2001-09-18 10:31:34

From: Jens Maurer <Jens.Maurer_at_[hidden]>
>williamkempf_at_[hidden] wrote:
>>After looking closer at the proposed patch this simply won't work.
>>The Win32 implementation uses the DCL pattern, which for IA32
>>implementations is safe, so I'm not as concerned there.  But for
>>POSIX this implementation is going to run on architectures that use
>>aggressive reordering and the DCL simply isn't thread safe in that
>>case.
>
>One of the intents of Boost.Thread should be to show limitations of
>the current C++ memory model, and this certainly is one.  Of course,
>Boost.Thread also has to assume "correct" memory ordering when
>using synchronization primitives, e.g. changes performed in
>a given critical section on one CPU need to be globally visible
>when the next CPU enters that critical section.

This issue is well known and doesn't need to be highlighted by giving a 
(currently) incorrect implementation in Boost.Threads.  This is also an 
issue that can't be easily fixed by redefining the memory model.  For 
instance, I don't think you comprehend the problem here.  All 
synchronization primitives on all platforms insure the "correct" memory 
ordering within their scope.  With the DCL, however, you've violated the 
memory gaurantees given by the mutex's scope.  The DCL accesses shared data 
both before and after the lock/unlock scope of the mutex.  What can happen 
is that a thread checks the flag outside of the mutex scope and may see that 
the once routine has already been run and so skip the lock/unlock of the 
mutex entirely and then access memory which hasn't been synchronized between 
the two threads yet.  If we didn't check the flag outside of the scope of 
the mutex the memory issues wouldn't exist because we followed the memory 
visibility rules given us by the synchronization objects, but this would be 
an expensive implementation of the call_once() method.

>In the particular case with call_once(), the only problem happens
>when we transition "flag" back from true -> false, i.e. when an
>exception happened.  If no exception happens, other CPUs could
>still believe "flag" is false, enter the critical section, find
>out "flag" is true in reality (since we must assume that a
>critical section has proper memory ordering), and all is well.

We never transition the flag back.  Once "true" it's always true.  Again, 
you don't quite understand the issues at play here.  The "critical section" 
can only give us proper memory ordering gaurantees if we use it.  The DCL 
purposely avoids using it, and so on some platforms will give you erroneous 
results because of memory ordering issues.

>If an exception happened and "flag" transitions from true -> false,
>other CPUs could still believe that "flag" is true and avoid
>re-executing the target function.  That's a race.

The race problem, again, is with data that's modified by the once routine, 
not the flag itself.  However, it's *caused* by accessing the flag outside 
of the "critical section".

>The easy way to avoid that is to document that the target function
>of "call_once()" may not throw exceptions, or that it is considered
>executed even if it throws an exception.  I'm not sure which option
>is more useful.

I've already decided to document that it's non-throwing, but for different 
reasons.  Exceptions truly have nothing at all to do with the issue with 
DCL.  The DCL pattern is non-portable and unsafe even in C, where there is 
no concept of an exception.

Bill Kempf

_________________________________________________________________
Get your FREE download of MSN Explorer at http://explorer.msn.com/intl.asp