Re: [boost] [Serialization] Thread-safety of trunk code

From: Sohail Somani (sohail_at_[hidden])
Date: 2008-04-22 12:14:48

Robert Ramey wrote:
> Sohail Somani wrote:
[snip]
>> It appears you have addressed the thread-safety issue by ensuring that
>> all the mutable singleton objects get created before main. Is this
>> correct?
> 
> Correct.
> 
>> If so, then the problem is shifted to the implementation-specifics. I
>> see two things that need to be determined for a given compiler/OS:
> 
>> 1) If you dynamically load a dll/shared object (eg,
>> dlopen/LoadLibrary) does your loader guarantee that the global C++
>> constructors will be called? I recall some Unices that I've had the
>> misfortune of coming
>> across where this is not the case.
> 
> I have only tested with VC 7.1 and VC 8.0.  I have done this
> testing "by hand".  I've also included code which trapped if
> anything is created after main is called.

I don't blame you for testing "by hand". Is it possible to put a note 
for the user to test this on their own in the documentation before they 
use it at all? I feel a section in the "Implementation Notes" would be 
sufficient.

>> 2) Does the singleton<T> + static T reference hack work as expected on
>> the compiler?
> 
> I believe that the tests I used can be ported to any compiler - I don't
> recall if I incorporated the test code in all the tests.  One would have
> to check.

The export tests should be sufficient for this, I believe.

[snip]
>> Other than that, it looks good to me. Good job rewriting the dangerous
>> parts rather than patching in the thread-safety.
> 
> I  looked into this in some detail.  I concluded that a) it would be very
> hard to do and would muck the library and b) it would slow things
> down a lot.  It has changed my whole perception as to how/when
> to use multi-threading.

I agree. I had the same change in perception after reading your code. 
Only thing that worries me is the portability of it.

>> This is based on my reading and not actually testing the thread-safety
>> so your mileage may vary ;-)
> 
> Note that the really tricky part is dealing with dynamically loaded
> DLLS which are loaded after main is called. Static globals
> which are included in the DLL are constructed when the DLL is
> loaded and are destroyed when the DLL is unloaded.  Add to this
> the fact that a significant number of users have organized thier
> code so that some globals will appear in multiple DLLS and
> or the main module. The changes I made address all these
> issues.  Hopefully no one will appreciate it as it should "just work".

I hope I never have to deal with this other form of DLL hell myself ;-)

[snip]
> I think is mostly OK in spite of the shortage of testing.  But, given
> the nature of the changes - compiler dependent, os dependent,
> hard to test, non-standard compliant, etc. I expect
> a few thing to crop up so we'll see what happens.

Given that it is easy to run and report on regressions now, interested 
users really should be running them on their platforms of interest.

Of course, chasing down the authors when something goes wrong is another 
thing...

Anyway, I'm sure this update of the library is a welcome update!

-- 
Sohail Somani
http://uint32t.blogspot.com