< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] unsafe static Mutex object initialization


You wiki account is the same as your SVN account, it seems that your wiki edit privilege went missing.  I've added it back.

That https thing has burned me in the past before too. : )   The link referenced on the wiki does use https - but it would be good to add a note that indicates https is required for commiters.

Thanks for committing the fix!

Scott


On Fri, Apr 20, 2012 at 2:15 PM, Matthias Moetje <moetje@xxxxxxxxxxxx> wrote:

Hi Scott,

 

I commented and committed my change to the main branch now. I think this is more solid as in this case the dependency is more obvious and easier to explain.

 

After finding my login data, it took me 2h trying to find a way to specify my credentials in TortoiseSVN, only to find out that I just need to use https instead of http….damn…

 

I wanted to add a note about this to the WIKI, but it seems that my account was deleted ;-(

 

Best regards,

 

Matthias

 

 

From: slgodin@xxxxxxxxx [mailto:slgodin@xxxxxxxxx] On Behalf Of Scott Godin
Sent: Freitag, 20. April 2012 15:43
To: Matthias Moetje
Cc: Daniel Pocock; Aron Rosenberg; resiprocate-devel@xxxxxxxxxxxxxxx


Subject: Re: [reSIProcate] unsafe static Mutex object initialization

 

Hi Matthias,

 

I think your solution is reasonable.  Another way I can think of, would be to only try to lock mTlsDestructorsMutex in ThreadIf::tlsKeyCreate if is it not NULL.  Given that static initialization will happen in a single thread, locking the map during static init time isn't required.  Either solution should be well documented.  Are you able to commit one these solutions to SVN main?

 

Thanks!

Scott

 

On Thu, Apr 19, 2012 at 11:56 PM, Matthias Moetje <moetje@xxxxxxxxxxxx> wrote:

Hi everybody,

 

I just upgraded to the latest code version and got a problem with static initialization order.

 

In the message below, Daniel pointed out that the static initialization order is not reliable.

 

Please have a look at this call stack:

 

                xxx.tsp!takeLock(resip::Lockable & lockable, resip::LockType lockType)  Line 30 + 0x3 bytes       C++

               xxx.tsp!resip::Lock::Lock(resip::Lockable & lockable, resip::LockType lockType)  Line 39 + 0xd bytes       C++

               xxx.tsp!resip::ThreadIf::tlsKeyCreate(unsigned long & key, void (void *)* destructor)  Line 234               C++

               xxx.tsp!resip::LogStaticInitializer::LogStaticInitializer()  Line 87 + 0x10 bytes      C++

               xxx.tsp!resip::`dynamic initializer for '_staticLogInit''()  Line 426 + 0xd bytes      C++

               msvcr100d.dll!_initterm(void (void)* * pfbegin, void (void)* * pfend)  Line 873               C

               xxx.tsp!_CRT_INIT(void * hDllHandle, unsigned long dwReason, void * lpreserved)  Line 284 + 0xf bytes            C

               xxx.tsp!__DllMainCRTStartup(void * hDllHandle, unsigned long dwReason, void * lpreserved)  Line 506 + 0x11 bytes                C

               xxx.tsp!_DllMainCRTStartup(void * hDllHandle, unsigned long dwReason, void * lpreserved)  Line 476 + 0x11 bytes                C

 

LogStaticInitializer is instantiated and calls tlsKeyCreate. tlsKeyCreate tries to do

 

                Lock lock(*mTlsDestructorsMutex);

 

But at this time mTlsDestructorsMutex is NULL because

 

                static TlsDestructorInitializer _staticTlsInit;

 

has not been initialized yet. (TlsDestructorInitializer creates mTlsDestructorsMutex)

 

How should I work around this problem? I added  

 

TlsDestructorInitializer tlsDestructorInitializer;

 

as a private member to LogStaticInitializer and this made it work, but I am not sure if that's the best strategy...?

TlsDestructorInitializer seems to be used as a singleton (static TlsDestructorInitializer _staticTlsInit), but its implementation is obviously prepared to deal with multiple instances.

 

Another option to control static initialization order would be init_seg (http://msdn.microsoft.com/en-us/library/7977wcck%28VS.80%29.aspx), but it’s specific to Visual Studio…

 

Best regards,

 

Matthias

 

 

 

-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Daniel Pocock
Sent: Montag, 1. August 2011 00:54
To: Aron Rosenberg
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] unsafe static Mutex object initialization

 

 

 

I'm not convinced it is safe:

 

a) you can't be sure about the order that static initialization is done

- some other static initialization code in another translation unit depending on this static object could execute before it is in a good state

 

b) although it may not be normal practice within this code, can you be certain that no other static initializer starts a thread?

 

 

On 01/08/11 00:38, Aron Rosenberg wrote:

> I was referring to the ThreadIf stuff as safe since its indirectly

> only initialized during global static initialization. It would unsafe

> during regular runtime as you mentioned.

>

> Aron Rosenberg

> Sr. Director, Engineering,

> LifeSize, a division of Logitech

>

>

>

>

> On Sat, Jul 30, 2011 at 11:15 AM, Byron Campen <bcampen@xxxxxxxxxxxx> wrote:

>

>>  Actually, this too can be problematic, because you can get multiple

>> initializations of the Mutex if multiple threads try to construct the

>> same type of object at once. And, if you have any static functions

>> that try to use the mutex, you're sunk because it might not be

>> initialized. A better way to do this is the following:

>> 

>> In the header:

>> 

>> class FoobaJooba

>> {

>> static Mutex& getMutex()

>> {

>>  static Mutex mMutex;

>> return mMutex;

>> }

>> 

>> // Nobody should actually touch this, or use it to try to determine 

>> // whether the mutex has been initialized; this bool could be

>> anything // during static initialization. It exists solely to cause

>> getMutex()  // to be called sometime during static init.

>> static bool ensureInitialized;

>> };

>> 

>> In the implementation file:

>> 

>> bool FoobaJooba::ensureInitialized=(&initMutex()!=0);

>> 

>> This ensures that getMutex() is called sometime during static

>> initialization (it might be called multiple times, but this is ok,

>> since there is only one thread of execution), so multiple init

>> problems don't crop up, and it also ensures that if somebody tries to

>> access the mutex during static init _before_ ensureInitialized has

>> been initialized, the mutex is just initialized a little bit early.

>> 

>> Now, this is still not perfect; it doesn't prevent nastiness during

>> static _de_initilization.

>> 

>> Best regards,

>> Byron Campen

>> 

>> 

>> Best regards,

>> Byron Campen

>> 

>> The best way to handle this is actually to make the mutex be a static

>> pointer which gets allocated during a class constructor. See ThreadIf

>> class and mTlsDestructorsMutex and friends for the best way to implement this.

>> 

>> -Aron

>> 

>> Aron Rosenberg

>> LifeSize, a division of Logitech

>> 

>> 

>> On Thu, Jul 21, 2011 at 11:25 AM, Daniel Pocock <

>> daniel@xxxxxxxxxxxxxxxxxxxxx> wrote:

>> 

>>> 

>>> 

>>> 

>>> I notice a few places where a Mutex objected is constructed,

>>> typically as a static member of a class:

>>> 

>>> $ grep Mutex rutil/*.hxx | grep static

>>> Log.hxx:      static Mutex _mutex;

>>> Random.hxx:      static Mutex mMutex;

>>> Time.hxx:                  static Mutex mWrapCounterMutex;

>>> Time.hxx:                  static Mutex mMutex;

>>> 

>>> The order in which the constructors of these objects are called is

>>> non-deterministic

>>> 

>>> It is actually possible that an code section depending on the Mutex

>>> could be attempting to lock on the Mutex before the Mutex itself is

>>> initialized

>>> 

>>> 

>>> 

>>> For UNIX, it may be possible to initialize Mutex::mId =

>>> PTHREAD_MUTEX_INITIALIZER, or even define Mutex like so:

>>> 

>>> #define STATIC_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER typedef

>>> pthread_mutex_t Mutex;

>>> 

>>> and then in some file.cxx:

>>> 

>>> Mutex mMutex = STATIC_MUTEX_INIT;

>>> 

>>> 

>>> 

>>> However, the situation for Windows is not quite the same:

>>> 

>>> http://locklessinc.com/articles/pthreads_on_windows/

>>> 

>>> suggests a hack:

>>> 

>>> #define PTHREAD_MUTEX_INITIALIZER {(void*)-1,-1,0,0,0,0}

>>> 

>>> that depends on the internal structure of the Windows type

>>> CRITICAL_SECTION

>>> 

>>> Has anyone else got any thoughts on this, how it should be done on

>>> Windows, and any other platform considerations that I haven't raised?

>>> 

>>> 

>>> 

>>> _______________________________________________

>>> resiprocate-devel mailing list

>>> resiprocate-devel@xxxxxxxxxxxxxxx

>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

>>> 

>> 

>> _______________________________________________

>> resiprocate-devel mailing list

>> resiprocate-devel@xxxxxxxxxxxxxxx

>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

>> 

>> 

>> 

>

_______________________________________________

resiprocate-devel mailing list

resiprocate-devel@xxxxxxxxxxxxxxx

https://list.resiprocate.org/mailman/listinfo/resiprocate-devel


_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel