Bug 241465 - Race condition during initialization of static local variable objects
Summary: Race condition during initialization of static local variable objects
Status: RESOLVED FIXED
Alias: None
Product: kdevplatform
Classification: Developer tools
Component: language (show other bugs)
Version: git master
Platform: Compiled Sources Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-11 20:48 UTC by Krzysztof Nowicki
Modified: 2014-07-11 12:25 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Nowicki 2010-06-11 20:48:56 UTC
Version:           SVN (using KDE 4.4.3) 
OS:                MS Windows

Inside kdevplatform there are many cases where there is a global instance of a class held as a static local variable of a function that is supposed to return a reference to that global instance. In my case the problem occurred in duchain.

Example:

Utils::BasicSetRepository* RecursiveImportRepository::repository() {
  static Utils::BasicSetRepository recursiveImportRepositoryObject("Recursive Imports", &KDevelop::globalItemRepositoryRegistry());
  return &recursiveImportRepositoryObject;
}

For such variables the compiler generates a special 'guard flag', which ensures that the constructor is called only once. This solution is however not thread safe. In case several threads execute this function in parallel for the first time, two things can happen (depending on the compiler):
 - GCC - the guard flag is set after the constructor is called, which means the constructor could be executed more than once.
 - MSVC 9.0 - the guard flag is set before calling the constructor. This makes multiple constructions less likely, but on the other hand causes the function to return a reference to an instance, that is not yet initialized (it is being initialized in another thread).

On MS Windows this causes a crash when duchainify is executed in multithreaded mode. Adding a global QMutex + local QMutexLocker before the static variable solved the problem in my case.

Reproducible: Always
Comment 1 Andreas Pakulat 2010-06-11 21:16:16 UTC
Adding a mutex is not an option as these functions are supposed to be as fast as possible and locking is a very slow operation.
Comment 2 Milian Wolff 2010-06-11 21:30:26 UTC
we simply need to call them once in a defined way where no threads can exist, which will initialize that sooner and should solve the issues.
Comment 3 Krzysztof Nowicki 2010-06-11 21:51:23 UTC
What about using something like conditional mutex locker class. It would essentially be a QMutexLocker(), but with an additional argument telling it to lock or not:

bool globalInstanceInitialized = false;
QMutex globalInstanceCreationMutex;

SomeObject& globalInstance()
{
  QCondMutexLocker(&globalInstanceCreationMutex, !globalInstanceInitialized);
  static SomeObject globalInstance(params);
  globalInstanceInitialized = true;
  return globalInstance;
}

class QCondMutexLocker
{
public:
  QCondMutexLocker(QMutex *pMutex, bool lock) : pMtx(pMutex), lck(lock)
  {
    if (lock)
      pMutex->lock();
  }
  ~QCondMutexLocker()
  {
    if (lck)
      pMutex->unlock();
  }
};

When the function is called for the first time by multiple thread, all of them will (except one) will wait on the mutex, because lock == true. When the mutex is released, the instance will be already initialized. Any subsequent call will not use the mutex, because the global flag is set to true. The only additional overhead are 2 additional 'if' statements.
Comment 4 Krzysztof Nowicki 2010-06-11 21:55:10 UTC
Oops, missed the QCondMutexLocker variables:

private:
  QMutex *pMtx;
  bool lck;
}
Comment 5 Vadym Krevs 2010-06-11 21:58:45 UTC
pthread_once() is a better solution ...
Comment 6 Krzysztof Nowicki 2010-06-11 22:31:04 UTC
(In reply to comment #5)
> pthread_once() is a better solution ...

As long as you have pthreads available...

There is a Win32 port of pthreads for example, but the whole point of using Qt4 is to be platform independent.
Comment 7 Vadym Krevs 2010-06-11 22:55:48 UTC
All I'm saying (well, the pthread_once's man page) that using a mutex in this context won't solve the problem. Since other cross-platform libraries such as RogueWave's SourcePro provide a portable abstraction for thread-safe initialization, there is no technical reason why Qt could not do something similar. Until it does you have a choice to make - live with the bug or use an #ifdef.
Comment 8 Milian Wolff 2014-07-11 12:25:36 UTC
this should be fixed since quite some time now