Re: [boost] [thread] Proposal for thread safe initialization of local static and global objects

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2006-10-09 04:51:20

Raymond Haeb <ray.haeb_at_[hidden]> writes:

> Anthony Williams wrote:
>> Roland Schwarz <roland.schwarz_at_[hidden]> writes:
>> 
>>> Anthony Williams wrote:
>>>> Roland Schwarz <roland.schwarz_at_[hidden]> writes:
>>>>> class once_init does have a user declared constructor and a destructor!
>>>>> And this definitely is _not_thread safe.
>>>> Yes, agreed. That's why the operator-> also does a lazy_init().
>>>>
>>>> This is completely platform- and implementation-dependent code, precisely the
>>>> sort of thing that needs to be wrapped in a library, and not written by the
>>>> general public.
>>> I am not convinced yet, that this code is thread safe even on MSVC:
>>> You can happen to have multiple ctor calls waiting on
>>>              WaitForSingleObject(event,BOOST_INFINITE);
>>> in init_once, true?
>> 
>>> Then when the the event is signalled (after the object has been
>>> constructed) all pending ctors (possibly just having waited)
>>> return. Now if you look at the assembly code level, you will
>>> see that each will schedule a call to atexit, which makes your
>>> code end up with multiple calls to dtor. This is bad. isn't it?
>>> (Not to mention, that I think I found out that the call to atexit
>>> itself isn't thread safe :-(, at least for posix, don't know
>>> for MSVC yet.)
>> 
>> Only the thread(s) that runs the constructor will schedule atexit.
>
> both threads (may) call the contructor of once_init<A>. So it is
> possible, that the destructor of once_init<A> is scheduled multiple
> times for atexit. Or even atexit may be called multiple times
> simultaneously. 

Yes, that's true, which I accepted in the email that you're replying
to. Running the constructor multiple times (even concurrently) is fine, given
the way it's designed. The multiple destructor calls registered with atexit is
more of a problem with my initial code, but I stand by my statement:

>> Anyway, I'm sure this is fine if the destructor is changed to:
>> 
>>     ~once_init()
>>     {
>>         if(init_event)
>>         {
>>             (*this)->~T();
>>             CloseHandle(init_event);
>>             init_event=0;
>>         }
>>     }
>
> Not really, the compiler is allowed to rearrange anything in it as long
> as the observable behavior (according to a single threaded machine) is
> the same. Observable are only the calls to "library I/O functions" and
> accesses to volatile data. This means, that the compiler is allowed to
> rearrange anything inside the if clause as long as CloseHandle is called
> with the right value (and the other IO functions inside ~T() are called
> in the right order and with the right parameters). And even if you make
> init_event volatile it does not change much: init_event=0 is after
> CloseHandle but not necessarily after ~T().

This destructor will only be run from one thread (the thread processing the
atexit calls). It may be run multiple times, but the if(init_event) and the
init_event=0 take care of that, even if they are reordered within the
destructor.

This code is intended to be platform- and implementation-dependent. I have
investigated how MSVC 7.1 and MSVC 8.0 work in this instance, and believe that
my code will work on those compilers.

Anthony
-- 
Anthony Williams
Software Developer
Just Software Solutions Ltd
http://www.justsoftwaresolutions.co.uk