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

Subject: Re: [boost] [Boost.Serialization] Crash in current master (1.68)
From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2018-07-11 16:32:15

> The problem is that I don't see any connection between your proposed 
> change and the failed test.  I'm not disputing that your change makes 
> a problematic symptom go away.  But I haven't been able to invest 
> enough time to really understand why that might be so.
Please do not focus on that currently checked in test (test_dll_load or 
what it was) because that one is to complex to reason about.
Please check the very basic singleton interface tests I even added as a 
separate PR (https://github.com/boostorg/serialization/pull/110) which 
clearly shows, that the current implementation is flawed.
Additionally I have (empiric) evidence that destruction order is 
unreliable (see the test I sent around in this ML).
So what my proposed change is, is simply:
 Â Â Â Â  a) fix the bug in the implementation shown by #110 (should be 
indisputably the right thing to do) and
 Â Â Â Â  b) check if the singleton accessed by a singleton-destructor is 
still alive (equivalent of `if(foo) foo->bar()`)
I don't see how this fixes a symptom only. It tackles the root causes: A 
defect in the code and a use-after-free caused by the unexpected 
destruction order (although the cause for that is unclear and only know 
to be related to shared libraries).

I do acknowledge your lack of time and because the fix is so 
logical/straight forward I asked for another reviewer to relieve you 
from that or help out.
> The whole issue of dynamically loaded DLLS/shared libraries has been a 
> festering sore for at least 20 years now.  It's undefined by the 
> standard and the committee has not seen fit to address it. Sorry it 
> just takes time to get this right.
I also do understand this. But I have proof that your latest change 
causes a (possible) crash instead of a (certain) memory leak and hence 
ask you to revert that change for 1.68 or the library will be unusable 
for some users. Once there is more time, the issue can be fully resolved.

Alex Grund