0

I'm curious about the following code:

class MyClass
{
public:
   MyClass() : _myArray(new int[1024]) {}
   ~MyClass() {delete [] _myArray;}

private:
   int * _myArray;
};

// This function may be called by different threads in an unsynchronized manner
void MyFunction()
{
   static const MyClass _myClassObject;
   [...]
}

Is there a possible race condition in the above code? Specifically, is the compiler likely to generate code equivalent to the following, "behind the scenes"?

void MyFunction()
{
   static bool _myClassObjectInitialized = false;
   if (_myClassObjectInitialized == false)
   {
      _myClassObjectInitialized = true;
      _myClassObject.MyClass();   // call constructor to set up object
   }
   [...]
}

... in which case, if two threads were to call MyFunction() nearly-simultaneously, then _myArray might get allocated twice, causing a memory leak?

Or is this handled correctly somehow?

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234
  • 1
    Lots of dupes including http://stackoverflow.com/questions/246564/what-is-the-lifetime-of-a-static-variable-in-a-c-function –  May 30 '10 at 17:12
  • 1
    This is probably also relevant: http://stackoverflow.com/questions/1661529/is-meyers-implementation-of-singleton-pattern-thread-safe – Dan O May 30 '10 at 19:46

2 Answers2

1

There's absolutely a possible race condition there. Whether or not there actually is one is pretty damn undefined. You shouldn't use such code in single-threaded scenarios because it's bad design, but it could be the death of your app in multithreaded. Anything that is static const like that should probably go in a convenient namespace, and get allocated at the start of the application.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • -1 for this. Putting statics in functions is the foundation for the singleton in C++, and sometimes you don't want to pay for the overhead each program run for constructing a global. Additionally, construction order for globals is undefined between translation units, while you can ensure objects are constructed in the right order using local statics. It's not "bad design". – Billy ONeal May 30 '10 at 17:40
  • The fact that it is the basis of a design pattern does not make it not bad design. Especially since, as noted, it is not portable to multiple threads. – Puppy May 30 '10 at 17:58
  • I never said that it's place as part of a pattern made it good design. But that does not mean it is bad either. As for multiple threads, it's perfectly portable depending on your compiler. Since any type of threading is compiler-specific, talking about any type of threading at all is inherently non-portable, so I see no basis for calling such a thing unportable. – Billy ONeal May 30 '10 at 18:47
  • I was discussing portable to multiple threads. Not portable between compilers. And the OP is perfectly right- there's nothing stopping bugs happening here. Some compilers support thread-local storage, some don't, but if the compiler is a pure single-threaded beast who has never heard of and provides no functions for multi-thread, it would be perfectly fine for them to error on this. – Puppy May 30 '10 at 19:07
0

Use a semaphore if you're using multiple threads, its's what they're for.

Matt S
  • 1,767
  • 8
  • 9