Re: [boost] [Boost.Serialization] Crash in current master (1.68)

Subject: Re: [boost] [Boost.Serialization] Crash in current master (1.68)
From: Robert (r.firl_at_[hidden])
Date: 2018-07-02 13:30:45

On 7/2/2018 2:22 AM, Alexander Grund via Boost wrote:
>> To put this a simpler way:
>>
>> If you want to be able to use Boost.Serialization as a static library 
>> within shared libraries, then you must make absolutely certain that 
>> there are exactly zero #includes or references of any kind to 
>> Boost.Serialization's header files in any of the public header files 
>> of the shared library.
>>
>> If everything compiles after that, then you should be ok.  This 
>> usually requires making use of PIMPL techniques.
> Please have a look at my test: The interfaces of the shared libraries do 
> NOT contain anything boost related. Only very simple C functions that 
> are called to make sure, the singletons within Boost are instantiated. 
> So your preconditions are met, yet the failure occurs.
> 
> Possible use case: You use Boost.Serialization in your library and use 
> another shared library which uses Boost.Serialization itself, but 
> completely independently. Nothing about it in the interface, but it 
> would still crash.

Although crashing is not observed with the slightly older Boost releases 
(e.g. 1.66, 1.65.1, 1.64, etc.), intentionally isolating 
Boost.Serialization as Gavin describes fails, with the already described 
lack of destructor invocations and memory leak.

> 
>  >>This is due to the bug I described and fixed in 
> https://github.com/boostorg/serialization/pull/105.
>  >>An extract from there only showing the bug with `is_destroyed` is 
> https://github.com/boostorg/serialization/pull/110
>  >>
>  >>In the current state I'm strongly against including the latest master 
> from Boost.Serialization. It would be better, to include the Boost 1.67 
> version of it which "only" contains a memory leak.
>  >
>  >We differ on the cause, the fix and the work around.  Objection noted.
> 
> Ok let me put this differently and lets see how far we can agree:
> 
> 1. Destruction order may or may not be as intended (For single binaries 
> destruction order is inverse of construction, but my test shows, that 
> this does not hold for this shared library of static library mix)
> 2. Singletons of Boost.Serialization have a method `is_destroyed` which 
> returns whether one can still access the singleton or not
> 3. `!is_destroyed` is asserted by the destructors of singletons 
> referring to other singletons
> 4. 3. will fail if 1. does not hold, the program will then crash (or UB)
> 5. if 1. is true, the crash can be avoided by checking `is_destroyed` 
> before accessing the singleton (not doing so if destroyed)
> 6. `is_destroyed` is broken, hence the assertion is unreliable and we 
> get crashes where we would first get assertion failures in debug builds
> 
> Hence the steps to fix this:
> First and foremost: Test and fix `is_destroyed`
> Second: implement 5.
> 
> While 1. is an assumption that is disputable I think the rest is not. 
> Correct me here, if I'm wrong. Hence 
> https://github.com/boostorg/serialization/pull/110 which adds the test 
> for `is_destroyed`.
> Even if there is another explanation for 1. what is wrong with 
> implementing 5.? It will always be safe and code like 
> `if(!foo.is_destroyed()) foo.use()` is quite idiomatic C (`if(foo) 
> foo->use()`)
> 

I completely agree with your approach.  I also agree that having 
crashing and/or undefined behavior (UB) is not acceptable for any 
release, let alone the upcoming 1.68.

--Robert

> Alex Grund
> 
> _______________________________________________
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost