Static object initialization and thread safe  
Author Message
ChinaGirl





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

I have ran into a situation that is very werid. Could anyone help me

This a C++ application built and debugged on Visual Studio .Net 2005. Basically I'm implementing a singleton class using double check locking pattern. I have read many arcticles about how this pattern is not thread safe, but my problem is slightly different. Here is the pseudo code:

MyMutex.h/cpp

class MyMutex {

public:
...

Lock() {

...

}

Unlock() {

...

}
...
};

ThreadMutex.h/cpp
class ThreadMutex {

public:
ThreadMutex() {
...
_lock = new MyMutex();
...
}

Acquire() {
...
_lock->Lock();
...
}

Release() {
...
_lock->Unlock();
...
}

...
private:
MyMutex* _lock;
}


MyObject.h

class MyObject {

protected:
MyObject();
...

public:
static MyObject Instance()

private:
static MyObject* _instance;
}



MyObject.cpp


MyObject* MyObject::_instance = 0;

MyObject* MyObject::Instance() {

if (_instance != 0) { //line 1
return _instance;
}

static ThreadMutex _instanceLock; //line 2

_instanceLock.Acquire(); //line 3

if (_instance == 0) {

_instance = new MyObject();
}

_instanceLock.Release();

return _instance;
}

MyObject will get accessed during application starting up (by two static object constructor) from two threads (yes the application has two threads even before main is started, one is main thread, another thread is created by a static object constructor). About 1 of 50 times, the application will crash at MyMutex::Lock() because "this=0x00000000"

When I debug on Visual Studio .Net 2005, it seemed to me what happened is the following in the order:

1. Thread 1 enters Instance() method at Line 1

2. Thread 1 proceeds to Line 2 because _instance =0, and Thread1 will initialize static object _instanceLock

3. Before _instanceLock is fullly constructed by Thread 1, Thread 1 is preempted by Thread 2

4. Thread 2 enters Instance() method at Line 1

5. Thread 2 proceeds to Line 3 because _instance =0 and static object _instanceLock is considered to be initialized at this time (decided by some sort complier flag )

6. Thread2 calls _instanceLock.Acquire(), which calls _lock->Lock(), but _lock is null at this time, since _instanceLock hasn't been fully constructed.

7. Application crashes

I think this can explain why this crashes happens occasionally.

I try to fix this by changing the function scope static _instnaceLock to class scope static, but found _instanceLock is still null when the first thread is trying to call _instanceLock.Acquire() and which of course crashes the application. This is understandable since we don't know the order of static objects get initialized during application starting up.

My another try is to use MyMutex directly in the Instance() method, which decreases the chances that threads get preempted during the construction of the _instanceLock object. So far I haven't seen crash yet with this change. But I think it's still not totally safe.

Could anyone tell me if my understanding about this crash is correct And is there any good approach to solve this problem

Thanks a lot!



Visual C++15  
 
 
valikac





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

Unavoidable. This type of race condition is difficult to solve. Initialize the kernel objects before spawning the threads, or deny access until they are initialized.

valikac

 
 
einaros





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

You should have a look at http://blogs.msdn.com/oldnewthing/archive/2004/03/08/85901.aspx. Scoped static initialization is generally not the way to go for these cases. You can get around the woes by writing your own spinlock in the head of the Instanace() function, but that will really be more trouble (and even more potential pitfalls) than it's worth.



 
 
Holger Grund





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

Of course, it's perfectly possible to get decent initialization of local statics with dynamic initialization. That there is a requirement in the C++ for what happens on concurrent threads is news to me. Anyway, the Generic C++ ABI solves the problem with a really trivial set of initialization guards. The bottom line here is, it is certainly not a good idea in VC, but that's not a standard requirement but just a crappy implementation. BTW: I'm not sure if that has been mentioned anywhere, but usually a worse problem with local statics is that cleanup will often run under the loader lock.

Anyway, it's probably a good idea to initialize things before spawning multiple threads. And that's definitely possible in a mixed executable as well (in fact, I'm very certain that native global initializers run before any static constructors in a mixed - or pure image). If that doesn't work for you, there's still static initialization which you can rely on. Along with interlocked access one can easily construct a trivial lock. Just take a look at the (_)Interlocked(Compare)Exchange function/intrinsic.

-hg


 
 
einaros





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

1. Thread 1 enters Instance() method at Line 1

2. Thread 1 proceeds to Line 2 because _instance =0, and Thread1 will initialize static object _instanceLock

3. Before _instanceLock is fullly constructed by Thread 1, Thread 1 is preempted by Thread 2

4. Thread 2 enters Instance() method at Line 1

5. Thread 2 proceeds to Line 3 because _instance =0 and static object _instanceLock is considered to be initialized at this time (decided by some sort complier flag )

6. Thread2 calls _instanceLock.Acquire(), which calls _lock->Lock(), but _lock is null at this time, since _instanceLock hasn't been fully constructed.

7. Application crashes

I'd go as far as to say that your main problem here isn't static initialization order. While you will be in trouble without a proper spinlock (preferably with acquire/release semantics) around the initialization of a function static lock object, I consider the worst issue to be the double checked locking like approach. As you yourself see, _Instance isn't always fully initialized, and without the proper memory barrier you cannot guarantee it to be. I could ramble on about it here, but there's really no way I could do that any better than Meyers and Alexandrescu did in http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf. A must-read, really.

I try to fix this by changing the function scope static _instnaceLock to class scope static, but found _instanceLock is still null when the first thread is trying to call _instanceLock.Acquire() and which of course crashes the application. This is understandable since we don't know the order of static objects get initialized during application starting up.

If the lock object is moved to class scope, and Instance() isn't called as part of the initialization of another static object, your woes are most certainly caused by the problems described in the article above.



 
 
ChinaGirl





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

Thanks for everyone's reply!

I did read many articles about how double check lock pattern is not thread safe. That's something I wasn't aware of when I chose to use it in my code. But I still think the crash hasn't reached creating the singleton instance part yet, but in creating lock. My understand is this lock is a singleton itself per singleton object (the real singleton object we want to create).

So I think I have two problems to solve now, how to make creating lock thread safe, and how to make the instance thread safe under the circumstances that application is starting up, multiple threads have been created before main(), and multiple threads try to create instance object.

-CG


 
 
Holger Grund





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

As I said before, initialization of local static is safe, so long as its static initialization. For instance, something like:

void foo() {
static i
nt i = 0;
}

Typically, compilers will statically allocate an integer in the initialized data segment of the image. So void foo()::i is guruanteed to be initialized to 0 before any code is executed. In case you wonder, the compiler will emit i in a section called ".bss" which ends in the ".data" section of your EXE/DLL and is marked as uninitialized data (that is a bit misleading as it is actually zero-initialized). The Windows loader will read the headers of the image, create the in-memory sections and initialize the accordingly before passing control to the program.

Hence, it is safe to rely on i being 0 on program startup. That is different from dynamic initialization which may be delayed to the first call which is problematic in the presence of multiple threads.

Now, with that background you can easily construct a trivial lock. Something like this should do the trick (I'm too lazy to try it out now, but it should give you the basic idea):

void guarded_init_function() {
enum { uninitialized, initializing, initialization_complete };
static volatile long init_guard = uninitialized;

long state;

while ( ( state = InterlockedCompareExchange( &init_guard
, initializing
, uninitialized ) ) == initializing )
Sleep(50);


if (state == initialization_complete ) return;

try {
actual_init();
} catch ( ... ) {
InterlockedExchange( &init_guard, uninitialized );
throw;
}
InterlockedExchange( &init_guard, initialization_complete );
}

}

-hg


 
 
einaros





PostPosted: Visual C++ Language, Static object initialization and thread safe Top

If I was to implement a singleton using the locking seen in the original post, I would do it along the lines of the following

=== MyObject.h ===

class MyObject
{
public:
    static volatile MyObject* Instance();

protected:
    MyObject();

private:
    static volatile MyObject* volatile _instance;
    static ThreadMutex _instanceLock;
};

=== MyObject.cpp ===

volatile MyObject* volatile MyObject::_instance = 0;
ThreadMutex MyObject::_instanceLock;

volatile MyObject* MyObject::Instance()
{
    if(_instance != 0)
    {
        return _instance;
    }

    _instanceLock.Acquire();

    if(_instance == 0)
    {
        MyObject* tmp = new MyObject();
        // Make sure that tmp is fully constructed before _instance is set to tmp,
        // using the full memory barrier implemented by InterLockedExhcangePointer
        InterlockedExchangePointer(&_instance, tmp);
        _instance = tmp;
    }

    // The release must include a memory barrier (either full, or release semantics),
    // to make sure that the changes done to _instance is seen by all threads on all
    // CPU's before the lock is released
    _instanceLock.Release();

    return _instance;
}