Menu

#196 TTlsContainer is not thread-safe

6.40
closed
5
2014-07-24
2011-12-14
No

The documented usage of TTlsContainer is as a function-local static variable, whose initialization is not thread-safe.

This problem not only pertains to the documentation; TTlsContainer is used, in the way documented, throughout OWLNext:

:::text
source\owlcore\docking.cpp(2433):  static TTlsContainer<TDitherBrush> __DitherBrush;
source\owlcore\locale.cpp(88):  static TTlsContainer<_TLocaleCacheList> __LocaleCacheList;
source\owlcore\memory.cpp(156):  static TTlsContainer<__TFixedAlloc> __FixedAlloc;
source\owlcore\memorydc.cpp(66):  static TTlsContainer<TMemDCCache> __MemDCCache;
source\owlcore\owl.cpp(92):  static TTlsContainer<TInstance> container;
source\owlcore\picker.cpp(1618):  static TTlsContainer<TCustomColors> __CustomColors;
source\owlcore\uiface.cpp(64):  static TTlsContainer<TUIFaceData> __UIFaceData;
source\owlcore\uihandle.cpp(40):  static TTlsContainer<THatchBrush> __HatchBrush;
source\owlcore\window.cpp(1341):  static TTlsContainer<TCacheEntryStr> __CacheEntry;


In addition, the implementation of TTlsContainer is flawed. To store the contained object it indexes into the TTlsDataManager storage. The index determined by the first thread is then used for all subsequent threads. This is a problem because TTlsDataManager keeps an independent storage array (TLocalData) for each thread. For some threads, the selected index may already be occupied, leading to the existing unrelated object being returned and coerced into the contained type, instead of a new one created. This will cause undefined behaviour.

Related

Bugs: #620
Wiki: Multi-threading_and_OWLNext

Discussion

  • Vidar Hasfjord

    Vidar Hasfjord - 2011-12-14

    Test program that demonstrates the index bug

     
  • Vidar Hasfjord

    Vidar Hasfjord - 2011-12-18

    This issue was fixed in revision 1233:
    http://owlnext.svn.sourceforge.net/viewvc/owlnext?view=revision&revision=1233

    Rewrote TTlsContainer, thus eliminating the index-bug. Forced early initialization (at program start-up) throughout OWLNext of all TTlsContainers using the Meyers Singleton idiom, thus avoiding the race condition on the initialization.

    Notes:

    • this fix assumes that program start-up runs single-threaded. If child threads are started before OwlMain runs, and call into OWLNext code, then all bets are off.

    • since the initialization of the TTlsContainer singletons are now forced to happen during program start-up, a contained object will always be allocated for the main thread, even if the main thread does not use the singleton itself (i.e. only used by child threads). It is possible to avoid this by refactoring the client code, should it become a problem.

     
  • Vidar Hasfjord

    Vidar Hasfjord - 2014-04-26
    • Status: pending --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB