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-08 15:18:56

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.

The set of the "constructor has been run" flag happens before the constructor
is run, so this should generally be avoided. It's not an atomic instruction,
though, so it's possible that multiple constructor runs will happen. That's OK
in itself, because the constructor runs the same init routine as the
lazy_init, and will only actually do the init once, even if called multiple
times concurrently. More concerning is the multiple atexit calls, since that
means that the destructor also has to be capable of being run multiple
times. They won't be concurrent, though, so there's no locking worry. Just
setting the init_event handle to NULL after closing it will do fine.

By checking the CRT source, I can see that atexit is thread-safe in MT builds
on MSVC.

>> On MSVC 7.1, the compiler will call the constructor in the first thread that
>> starts the function, and not any others.
>
> Sorry I cannot see this. Why wouldn't the others beeing called while
> construction is underways?

The code looks like this:

        mov	eax, DWORD PTR ?$S1@?1??f@@YAXXZ_at_4IA
        and	eax, 1
        jne	SHORT $L86349
        mov	ecx, DWORD PTR ?$S1@?1??f@@YAXXZ_at_4IA
        or	ecx, 1
        mov	DWORD PTR ?$S1@?1??f@@YAXXZ_at_4IA, ecx

which sets the flag once it's been decided to call the constructor before it
actually calls it, so it doesn't get called again. Of course, as I pointed out
above, this is not atomic, so isn't reliable. Thanks for making me look
closer.

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;
        }
    }

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