tags:

views:

367

answers:

7

Usually, when I try to initialize a static variable

class Test2 {
public:
    static vector<string> stringList;
private:
    static bool __init;
    static bool init() {
        stringList.push_back("string1");
        stringList.push_back("string2");
        stringList.push_back("string3");

        return true;
    }
};

// Implement
vector<string> Test2::stringList;
bool Test2::__init = Test2::init();
  1. Is the following code thread safe, during static variable initialization?
  2. Is there any better way to static initialize stringlist, instead of using a seperate static function (init)?

Although the initialization shall happen before main function (Hence, there can be no threads to simultaneous access the init), my concern is that :

  1. I have an exe application.
  2. My exe application will load a.dll, b.dll and c.dll
  3. a/b/c.dll, in turn will load common.dll. The above code are inside common.dll
  4. I had already verify. Since 3 dll are within single process, they will be referring to the same static variable (vector).
  5. In this case, to prevent 3 dlls from simultaneous access init (Can I view them as 3 threads? Although doesn't make sense at first thought), for the init function, shall I use a critical section to protect it?

I am using Windows XP, VC6 and VC2008 compiler.

+6  A: 

Is the following code thread safe, during static variable initialization?
That depends entirely upon your compiler. The standard says absolutely nothing about multithreading.

Is there any better way to static initialize stringlist, instead of using a seperate static function (init)?
First, you need to remove the __s (That's two underscores) in front of the name; any name beginning with __ is reserved according to the standard. (There are some situations where a single underscore is not allowed either -- it's best just to avoid leading underscores on names altogether)

That depends entirely on whether the vector needs to change during its runtime. If not, you're probably off just using a builtin array:

static const char *strings[] = {
    "string1",
    "string2",
    "string3"
};
Billy ONeal
@David Thornley: Thank you for the correction. I have edited the answer.
Billy ONeal
How about comment from Ken Bloom? In practical, how possible is bad thing to happen? (Multiple threads invoke init?)
Yan Cheng CHEOK
@Yan Cheng CHEOK: As I said, that depends completely upon your compiler. Yes, static initialization occurs before main. That does not mean things are inherently "thread-safe", because then multiple threads are looking at the same object. Simply put, there's nothing anyone can say about this without knowing your compiler and platform.
Billy ONeal
I am using Windows XP, and both VC6 and VC2008 compiler.
Yan Cheng CHEOK
Then AFAIK you shouldn't have to worry about threading issues with global statics. Local statics are another matter but that's another show ;)
Billy ONeal
I am having an exe, which will load a.dll, b.dll and c.dll. In turn, a/b/c.dll will all load a common library named common.dll. The above static code is in common.dll. Since it is within a single process, a/b/c.dll will sharing the same static data. In that case, is static initialization for above case is a concern?
Yan Cheng CHEOK
I wouldn't be so sure a b and c are sharing the same data. The standard doesn't say anything about DLLs either. I'm unsure. You might want to edit that into your question so that you get some better answers than mine :)
Billy ONeal
@Yan I've deleted my answer, because I realized that you could start threads from a constructor which could be called by a static initializer. You could also `dlopen()` a library after main starts, and its static initializers would be called at that time.
Ken Bloom
I would make them `static const char* const`, but otherwise, yes; this.
jeffamaphone
A: 

it is not thread-safe because several threads can call it simultaneously and you will have stringList initialized twice. Use thread synchronization.

Andrey
A: 

No code using identifier names with a double underscore can be considered safe for anything, as it violates the Standard's rules on names reserved for the implementation. The same is true for a name with a leading underscore followed by an uppercase letter, but other names beginning with an underscore may be used, as long as they're not in the global namespace. See the Standard, 17.4.3.1.2. These restrictions are continued in the latest Final Committee Draft for C++0x, under 17.6.3.3.2.

David Thornley
This is a worthwhile code correctness point for C++, but it's unrelated to the question.
Ken Bloom
@Ken Bloom: I disagree. If the code were perfectly thread-safe in every other way, it would still be unsafe since declaring a `__init` variable may overwrite something important. I consider it one legitimate concern here (and obviously there are more).
David Thornley
+2  A: 

Local static initialization is not thread safe. See http://blogs.msdn.com/oldnewthing/archive/2004/03/08/85901.aspx.

Global static initialization is usually thread safe. You code should work OK.

I normally use a separate init function, and use boost::call_once to ensure that it is called only once.

boost::once_flag boost_once_flag = BOOST_ONCE_INIT; 
boost::call_once(init_static_var, boost_once_flag);
Sherwood Hu
A: 

Check out my quick example that demonstrates that your container isn't thread safe.

#include <boost/thread.hpp>
#include <vector>
#include <string>

class NotThreadSafeContainer {
   static std::vector<std::string> strings;
   static bool is_initialized;

public:
   NotThreadSafeContainer() {
      if (!is_initialized) {
         is_initialized = true;
         strings.push_back("string1");
         strings.push_back("string2");
         strings.push_back("string3");
         strings.push_back("string4");
         strings.push_back("string5");
      }
   }
};

bool NotThreadSafeContainer::is_initialized = false;
std::vector<std::string> NotThreadSafeContainer::strings;

void thread_routine() {
   while (true) {
      // Wow! A container
      NotThreadSafeContainer ts_container;
   }
};

void main() {
   // Uncomment this to remove thread desync errors
   // boost::once_flag once_flag = 0;
   // boost::call_once(once_flag, thread_routine);

   // Start some threads
   boost::thread t1(thread_routine);
   boost::thread t2(thread_routine);
   boost::thread t3(thread_routine);

   NotThreadSafeContainer ts_container;
   bool SET_BREAKPOINT_HERE;
}
Kotti
But that's difference. My initialization is happen before main.
Yan Cheng CHEOK
Oops :) Never mind then, the only problems you could face is when you have many static data initialized this way (which rely on each other), because the order is undefined.
Kotti
A: 

If you need access to these strings prior to main running, this may or may not appear to work depending on how the compiler/linker order initialization.

Better would be to either:

  1. Call init from main so you know the program state when it's populating.
  2. Use a singleton whose constructor populates the strings into a non-static member vector. Then when you first instantiate the singleton (to remove possibility of initialization problems, after static initialization) it will add the strings.
Mark B
+1  A: 

I asked a similar question a while back:

LoadLibrary and Static Globals

When it comes to DLLs, static initialization and the call to DllMain is bracketed by an internal critical section, so they are thread-safe. A second thread will wait until the first is done before it loads the DLL.

So in short, your static init is safe.

Marc Bernier