On 10/16/18 7:23 AM, Peter Dimov via Boost wrote:

Alexander Grund wrote:

...

https://github.com/boostorg/serialization/pull/111/files

which does indeed crash

https://travis-ci.org/boostorg/serialization/jobs/441654483#L2546

That's easy enough to follow and does show a legitimate problem, in my opinion, although Robert refuses to acknowledge it for some reason.

The test does not show anything at all. It's ill conceived. A similar test could be made and it might be useful, but if I do all I'll get is a lot of heat from people who haven't take the time to understand what is actually going on.

the only failures are with Clang and they happen without your patch as well.

My version of the patch captures the essence of the PR while retaining the original intent of the code. It's much simpler and alters many less files. I have no idea why I am being criticized for making this patch.

In either case, it has to be Robert who approves and applies the patch, as "the community" doesn't understand the library as well as he does. (I certainly do not.)

FWIW - I often forget how the library works. Sometimes an issue comes up with code that hasn't been visited in over a decade. In such cases I have to review the documentation, tests and source code. I think I'm the only one who actually does this. Robert Ramey

On 10/16/18 8:11 AM, Peter Dimov via Boost wrote:

Robert Ramey wrote:

...

On 10/16/18 7:23 AM, Peter Dimov via Boost wrote:

...

https://github.com/boostorg/serialization/pull/111/files

which does indeed crash

https://travis-ci.org/boostorg/serialization/jobs/441654483#L2546

That's easy enough to follow and does show a legitimate problem, in my > opinion, although Robert refuses to acknowledge it for some reason.

The test does not show anything at all. It's ill conceived.

What is ill-conceived about it? It's a minimal example of two shared libraries each using Serialization. As Alexander says, it's a simplified version of a crash they had in one of their projects.

First of all, the test changes every time I look at it. It's hard to follow. On the last version I looked at - since no there is not BOOST_CLASS_EXPORT(class name) invoked, there are no indexed entries in the extended type info tables so get_key() should return null under any circumstances. It tests nothing. Worse, it betrays the fact that the author of the test, and anyone who doesn't see a problem with it, have read neither the document nor the source code. Rather than asking "What's ill conceived about it" better to ask - "What is this supposed to be testing?" Robert Ramey

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

On 10/16/18 8:32 AM, Peter Dimov via Boost wrote:

Robert Ramey wrote:

...

On the last version I looked at - since no there is not BOOST_CLASS_EXPORT(class name) invoked, ...

There are no class names in the test; it uses int and float.

...

Rather than asking "What's ill conceived about it" better to ask - "What is this supposed to be testing?"

The use case of two shared libraries each using Serialization, as already explained.

Well it's not testing that.

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

-----Original Message----- From: Boost On Behalf Of Robert Ramey via Boost Sent: Tuesday, October 16, 2018 5:22 PM

On 10/16/18 8:11 AM, Peter Dimov via Boost wrote:

...

Robert Ramey wrote:

...

On 10/16/18 7:23 AM, Peter Dimov via Boost wrote:

...

https://github.com/boostorg/serialization/pull/111/files

which does indeed crash

https://travis-ci.org/boostorg/serialization/jobs/441654483#L2546

That's easy enough to follow and does show a legitimate problem, in my > opinion, although Robert refuses to acknowledge it for some reason.

The test does not show anything at all. It's ill conceived.

What is ill-conceived about it? It's a minimal example of two shared libraries each using Serialization. As Alexander says, it's a simplified version of a crash they had in one of their projects.

First of all, the test changes every time I look at it. It's hard to follow.

If you keep saying that the test is bad / irrelevant, it is no wonder the author changes the code to improve it.

On the last version I looked at - since no there is not BOOST_CLASS_EXPORT(class name) invoked, there are no indexed entries in the extended type info tables so get_key() should return null under any circumstances.

If I understand this correctly, the purpose of the test is to show that the application crashes. In that case, the return value is completely irrelevant. If you think that crash is not the fault of Boost.Serialization (e.g. because it is used wrongly), then it would be helpful to explain what you believe to be the actual reason for the crash. What I don't know: Is there a travis build that shows that the test from Flamefire (https://github.com/boostorg/serialization/pull/111/files ) still fails with the changes introduced by Robert (I'm not sure when they were introduced)? Mike

On 10/16/18 8:49 AM, Mike Dev via Boost wrote:

...

-----Original Message----- From: Boost On Behalf Of Robert Ramey via Boost Sent: Tuesday, October 16, 2018 5:22 PM

On 10/16/18 8:11 AM, Peter Dimov via Boost wrote:

...

Robert Ramey wrote:

...

On 10/16/18 7:23 AM, Peter Dimov via Boost wrote:

...

https://github.com/boostorg/serialization/pull/111/files

which does indeed crash

https://travis-ci.org/boostorg/serialization/jobs/441654483#L2546

That's easy enough to follow and does show a legitimate problem, in my > opinion, although Robert refuses to acknowledge it for some reason.

The test does not show anything at all. It's ill conceived.

What is ill-conceived about it? It's a minimal example of two shared libraries each using Serialization. As Alexander says, it's a simplified version of a crash they had in one of their projects.

First of all, the test changes every time I look at it. It's hard to follow.

If you keep saying that the test is bad / irrelevant, it is no wonder the author changes the code to improve it.

If the test is bad/irrelevant, what else should I say. I have no problem if the author want's to improve his test. It's just not happening.

...

On the last version I looked at - since no there is not BOOST_CLASS_EXPORT(class name) invoked, there are no indexed entries in the extended type info tables so get_key() should return null under any circumstances.

If I understand this correctly, the purpose of the test is to show that the application crashes. In that case, the return value is completely irrelevant.

I haven't seen the application so I don't know why it crashes. I'm guessing that it might be related to the current (and recently improved state) singleton. I DO know test_exported_dll also crashes on one compiler configuration. I would like to fix this but so far no one has been able to provide a convincing change nor a test which points out where the problem might be.

If you think that crash is not the fault of Boost.Serialization (e.g. because it is used wrongly), then it would be helpful to explain what you believe to be the actual reason for the crash.

Which crash: in his app, in test_exported_dll, or in his test?

What I don't know: Is there a travis build that shows that the test from Flamefire (https://github.com/boostorg/serialization/pull/111/files ) still fails with the changes introduced by Robert (I'm not sure when they were introduced)?

Right - that are the exact same results which result from Alexander's original patch. That is, I merged in a simpler, less intrusive, less fragile version of Alexander's patch. And it "fixes" everything that Alexander's patch fixed. Clearly there are still some problems left - which are likely to be even more difficult to nail down. Note that to get this far has required 9 months of effort - part of which entailed including a breaking patch in to the release. Under ordinary circumstances, I would just fix Alexander's test and be done with it just to save time. But I've come to understand that it won't actually save time so I haven't done so. Robert Ramey

Mike

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

In short, yes. I understand your frustration, but the procedure is to square it off with the maintainer, then if you feel you're getting nowhere, escalate to the list. Basically what you've done, except that you're trying to discuss not the issue at hand, but "rights and responsibilities of maintainers", which is not going to help your issue, or anyone. I already escalated the issue to the list to avoid getting the crashing version into the last release. This obviously failed. My post here was after in the discussion of/in PR I got the impression

that I'm expecting to much of the maintainer: A review of my changes, I developed, tested and submitted. Someones else agreed to my point of view while others opposed. Hence I wanted clarification on the meta issue with a reference which I could bring on. Seems like the facts are against me in this case so I (or my changes) are in the mercy of the maintainer. As you mentioned it: #105 adds more CI #110 includes #105 and tests that singletons are eventually destructed and `is_destroyed` returns true in both use cases of the singleton #111 Is the condensed version of my MWE that shows the crash. Essence: Use 2 shared libraries linked against static boost and each uses a part of Boost.Serialization. The crash comes from a destruction order problem, that is currently unsolved and could be avoided if `is_destroyed` would return true, but it doesn't But Robert insists that #111 "does not show anything at all. It's ill conceived." He does not acknowledge that Boost.Serialization makes the application crash and does not bring any argument why it would be "ill conceived". I do not understand this. It obviously crashes. This is a fact easily reproducible by anyone on linux. If the test misuses C++ (UB, ...) or the library, then where and how? If not, why is it seen as "ill conceived"?

My version of the patch captures the essence of the PR while retaining the original intent of the code.

This is simply wrong. If it would #110 and #111 would pass. It fails to fix the `is_destroyed` function or the core problem with destruction order

It's much simpler and alters many less files.

My fix alters 1 file to fix `is_destroyed` and 4 more to remove a single line with an assertion which doesn't hold as shown in #111. Most of the changes are added reverts to an earlier, working version and comments on why and how stuff works, so it doesn't get accidentally broken by optimization/reduction efforts as happened in 1.65

I have no idea why I am being criticized for making this patch.

Because a) it combines unrelated changes, hiding the fix and b) pushing it w/o review by the original patch author who might have reasonable arguments why it isn't enough or "the essence of the PR". I learned it is your right to do that. But then again it is my right to to tell you that your claim is wrong. As I as the author of the patch might understand the patch better, it is quite possible, that I'm right, isn't it?

I really wonder how someone can show that much ignorance if all facts are against him. "#111 doesn't show anything", " I have no problem if the author want's to improve his test.  It's just not happening. " EVERYONE sees how it does show a crash. I explained that it is a MWE reproducer of a real application crash. So while the test may SEEM to not be doing much useful it DOES use Boost.Serialization and it DOES crash as described. There were NO arguments on WHY this test is invalid C++ code. Based on his comments (after months) I slightly changed the test and now I even created #128 which shall show a small, but valid and regular use of Boost.Serialization and it shows the exact same behaviour. Isn't it important to add tests for bugs found? Isn't it important to have these tests be as minimal as possible, so side effects are avoided? How can he still claim the test "doesn't show anything" if it clearly shows a construction/destruction problem IN the library? He also still fails to acknowledge the broken `is_destroyed` function. A good C++ programmer can see this from the code (just ask: When is is_destroyed set to false? When can this fail to happen?) and there is a test case where CI says "|Assertion failed: boost::serialization::singleton<plainSingleton>::is_destroyed()|" (see also comment https://github.com/boostorg/serialization/pull/110#issuecomment-429561420) Note how this was independently noticed by others too: https://github.com/boostorg/serialization/pull/110#issuecomment-417601919 And again and again he lacks reasoning:

It's based on a mis-understanding of how the serialization uses the singleton and of how the singleton works.

If it was, it would be easy to point out what mis-understanding this is. And it all boils down THAT the singleton is used. So even if the values in the singleton are bogus, all that is tested is the construction and destruction of the singleton. And the library DOES do that too. #127 clearly shows this.

...

As I as the author of the patch might understand the patch better, it is quite possible, that I'm right, isn't it?

It's possible - but after having considered that possibility, I've concluded that it's False

Again: No reasons, just claims. My claims are all backed by references to actual executions on CI and findings of other people coming to the same conclusion as me. Maybe it is time to put Boost.Serialization under CMT...

On Wed, Oct 17, 2018 at 6:12 AM James E. King III via Boost wrote:

Perhaps working towards removing the singleton from the implementation would be a worthy course of action. It has come up often in pull requests over the last few years, correct? What problem(s) does the singleton solve in the library, and could those issues be solved in a different way?

Great minds think alike! There was a nice discussion yesterday on Cpplang Slack of why the singletons exist, in the #boost channel (between me and Peter D) you should sign up and access the history: http://slack.cpp.al Regards

Robert Ramey wrote:

When I looked at the test my attention was immediately drawn to the following:

const char* impossible = "impossible"; return impossible != boost::serialization::extended_type_info_typeid<float>::get_const_instance().get_key();

I memory serves me, get_key() will return the address of string of characters inside the the address space of the DLL. This address is compared to the address of "impossible" which is an address inside the address space of the calling mainline application. I could not and cannot understand what is being tested here.

It's a dummy comparison whose only purpose is to make use of the return value to prevent the optimizer from removing the call entirely.

Robert Ramey wrote:

There is also an interesting issue of using float and int as type arguments. This is never been done before since no user of the serialization library would do this. It's an interesting new variable in the mix and offhand I don't really know if there are any consequences to this.

In response to these concerns, the author has since reformulated the test so that it only uses documented Serialization functionality: https://github.com/boostorg/serialization/pull/128/files

On 17/10/2018 04:15, Alexander Grund wrote:

As you mentioned it: #105 adds more CI #110 includes #105 and tests that singletons are eventually destructed and `is_destroyed` returns true in both use cases of the singleton #111 Is the condensed version of my MWE that shows the crash. Essence: Use 2 shared libraries linked against static boost and each uses a part of Boost.Serialization. The crash comes from a destruction order problem, that is currently unsolved and could be avoided if `is_destroyed` would return true, but it doesn't

Granted that I have not looked at the code or the PR in any detail (and thus this might be entirely off base): Conceptually "is_destroyed" is usually a bad idea. By definition, after the destructor has run the memory is free to get stomped on by other objects and thus you can't rely on is_destroyed to return any particular value. Trying to rely on it is UB. Applying this to singletons at global scope is both a blessing and a curse -- it reduces the likelihood that something else would actually stomp over the memory in practice, but also introduces a static destruction order indeterminacy issue. Usually the motivation to introduce this sort of thing is a side effect of having dangling pointers to deleted objects. In which case usually the best solution is to recognise that you actually have a shared ownership issue (where the object's actual lifetime is not what you thought it was) and start using shared_ptr/weak_ptr and/or having an object keep track of which singleton it is registered with rather than assuming that it will find the same ambient singleton later. Having said *that*, as previously discussed on the list, using a static library from multiple shared libraries in itself introduces an aliasing problem, where sometimes singletons aren't actually singletons -- and woe betide you if you try to pass an object that references one singleton across the library divide into the domain of the other singleton, because then you can have issues such as registering with one singleton and then trying to deregister from the other (which all by itself can be a source of dangling pointers). (FWIW, introducing dynamic libraries also increases the risk of dangling pointers, if a library can be unloaded while references to it survive outside.) As far as I am aware (when you are using static-from-dynamic), you will get these duplicated singletons at all times on Windows, and on Linux it will happen whenever you are compiling with -fvisibility=hidden (which appears to be true in this case given the build output) and if the singleton definitions don't use BOOST_SYMBOL_VISIBLE. I leave it to the rest of you to figure out whether this points at an underlying issue in Boost.Serialization or whether it's a usage error. Or both.

On 17/10/2018 13:22, Peter Dimov wrote:

Gavin Lambert wrote:

...

Conceptually "is_destroyed" is usually a bad idea.  By definition, after the destructor has run the memory is free to get stomped on by other objects and thus you can't rely on is_destroyed to return any particular value.  Trying to rely on it is UB.

It would be undefined to access the "is_destroyed" bool if it were a member of the singleton, but it's a function local static in Alexander's code, and it looks OK to me in that formulation. Static bools are never destroyed.

They are if they're in a shared library which is unloaded. :) Technically it's UB to access a global bool after "destruction" (just like any other object) and it could possibly be "destroyed" before the singleton due to unspecified global destruction order. Though you're correct that since bool has a trivial destructor it is *usually* not actually a problem in practice. (AFAIK a compiler is within its rights to zero or otherwise stomp the memory after "destruction", though most probably wouldn't bother, except perhaps as a diagnostic/sanitiser.) It's still a sign of a fundamental lifetime mismatch somewhere, though.

On 17/10/2018 14:08, Peter Dimov wrote:

Gavin Lambert wrote:

...

Technically it's UB to access a global bool after "destruction" (just like any other object)...

The only way to destroy a bool is to placement-new some other object over it, if I'm not mistaken. :-)

(Or, if it's in a union, you can assign to some other member, I suppose.)

The quotes were significant, not merely for emphasis.

On 17/10/2018 14:10, Steven Watanabe wrote:

That's not correct. If an object has static storage duration and a trivial destructor, then it is never "destroyed." Its storage exists "for the lifetime of the program" [basic.stc.static] and its lifetime ends when "the storage which the object occupies is reused or released" [basic.life]

Which occurs when the shared library that contains its storage is unloaded. The standard completely ignores the existence of shared libraries, so you're not on particularly stable ground. :)

On 17/10/2018 15:16, Peter Dimov wrote:

Gavin Lambert wrote:

...

...

That's not correct.  If an object has static storage duration and a trivial destructor, then it is never "destroyed." Its storage exists "for the lifetime of the program" [basic.stc.static] and its

On 17/10/2018 14:10, Steven Watanabe wrote: lifetime > ends when "the storage which the object occupies is reused or released" > [basic.life]

Which occurs when the shared library that contains its storage is unloaded.

How, specifically, are you planning to read a function local static from an unloaded library? You've already crashed way before that.

It's not inherently impossible for it to happen; the code can be inlined with only that variable being used as an external symbol pointing into unloaded memory. Which might not even crash when run on an OS that doesn't free the pages immediately (or at all). Again, though, this is a side track. My main point is that if you find yourself asking "has this object been destroyed?" then it's usually a sign of a more fundamental lifetime mismatch, and it's probably worthwhile exploring that.

Again, though, this is a side track.  My main point is that if you find yourself asking "has this object been destroyed?" then it's usually a sign of a more fundamental lifetime mismatch, and it's probably worthwhile exploring that.

This is true. But: a) is_destroyed is part of the public interface of Boost.Serialization (see the docs) and it doesn't return true when it should as shown by #110 even in the case of a simple executable. b) In the case I described and tested with #111 (and now even with #128) There are 2 shared libraries with their own instances of the singletons (this can be checked by debugging the ctor/dtor of them). The lifetime mismatch happens due to "some linux mechanism" destroying same types from different libraries at the same time invalidating the expectation of each library. Example (actually witnessed by printf-debugging) singleton<A> (0xlib1) singleton<B> (0xlib1) singleton<A> (0xlib2) singleton<B> (0xlib2) ~singleton<B> (0xlib2) ~singleton<A> (0xlib2) ~singleton<A> (0xlib1) ~singleton<B> (0xlib1) Note the switched order of the lib1 destruction. You can verify this yourself with #111 or #128. The problem arises because singleton<B> accesses singleton<A> in both its ctor and dtor but that has been destroyed and due to a) above this is not detected. We CAN try to find the reason for the mixup, OR we can rely on the public interface of singleton and detect it and handle it gracefully. The former is VERY hard, the latter is easy and actually it *is currently being done*!!! See https://github.com/boostorg/serialization/pull/105/commits/89d0910033527c34f04944d6499b788cb278761b All that is missing to avoid the crash is the fix for `is_destroyed`

On Wed, Oct 17, 2018 at 9:06 AM, Alexander Grund via Boost wrote:

b) In the case I described and tested with #111 (and now even with #128) There are 2 shared libraries with their own instances of the singletons (this can be checked by debugging the ctor/dtor of them). The lifetime mismatch happens due to "some linux mechanism" destroying same types from different libraries at the same time invalidating the expectation of each library. Example (actually witnessed by printf-debugging)

Using static libs in shared libs is a recipe for disaster isn't it? It's undefined behavior.. Does it make sense to try to 'work around' it on specific implementations? -- Olaf

On Thu, Oct 18, 2018 at 10:57 AM, Alexander Grund wrote:

...

Using static libs in shared libs is a recipe for disaster isn't it? It's undefined behavior..

Aren't shared libs per se undefined behavior? As far as I remember the

Implementation defined

standard does not say much about them and it happens easily "in the wild":

True

You might use Boost.Serialization yourself but also use a 3rd-party library which does use it for its own stuff.

...

Does it make sense to try to 'work around' it on specific implementations?

This is exactly what is happening:

Observed behavior on "specific implementations" is: - Destruction order on Windows+OSX is as expected, so no problems - On linux the order gets messed up. This gets detected by a dedicated

What does Linux 'say' about using static libs in shared libs?

Hence the "work around" is to detect and handle this:

My point is that it might not make sense to work around this as other issues are likely to pop up. -- Olaf

On 18/10/2018 08:52, Olaf van der Spek via Boost wrote:

On Wed, Oct 17, 2018 at 9:06 AM, Alexander Grund via Boost wrote:

...

b) In the case I described and tested with #111 (and now even with #128) There are 2 shared libraries with their own instances of the singletons (this can be checked by debugging the ctor/dtor of them). The lifetime mismatch happens due to "some linux mechanism" destroying same types from different libraries at the same time invalidating the expectation of each library. Example (actually witnessed by printf-debugging) Using static libs in shared libs is a recipe for disaster isn't it?

I was about to agree with you, but there is one very important use case - that of the application pluggin.  In that situation one would expect that as long as everything is built with -fvisibility-hidden so that each shared library is entirely self contained, then everything should really be fine. John. --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus

On Thu, Oct 18, 2018 at 5:17 PM Robert Ramey via Boost < boost@lists.boost.org> wrote:

[...] Now we call a plugin which includes another boost library - statically linked this time. The plugin will use the statically linked version of the C++ runtime library.

I don't think so. The plugin can link statistically against its dependencies, while still linking against the dynamically linked C++ runtime. I used to do that for Java extensions. The JNI library linked with all its C++ dependencies statically, so it's a single self-contained shared-library one could then java.lang.System.load() w/o having to mess with PATH or LD_LIBRARY_PATH, yet still linked against the dynamic runtime if I recall correctly (that's over 10 years ago). This is a legitimate use case IMHO... --DD

On 18/10/2018 08:52, Olaf van der Spek via Boost wrote:

...

On Wed, Oct 17, 2018 at 9:06 AM, Alexander Grund via Boost wrote:

...

b) In the case I described and tested with #111 (and now even with #128) There are 2 shared libraries with their own instances of the singletons (this can be checked by debugging the ctor/dtor of them). The lifetime mismatch happens due to "some linux mechanism" destroying same types from different libraries at the same time invalidating the expectation of each library. Example (actually witnessed by printf-debugging) Using static libs in shared libs is a recipe for disaster isn't it?

Rather than criticising Robert's maintainership, can we maybe spend more effort instead on how to best to solve his problem? I in the past have faced the problem where: 1. There must be a complex singleton instantiated in unknowable compilation units. 2. I don't control what weird linker flags the end user uses, or what combination of shared and static libraries they might use with me in each of them. 3. Code may use me during the shared library bootstrap and eviction process i.e. during static data init/deinit of plugins being dynamically loaded and evicted. The way I have solved this problem is brutal, but it works. In my singleton constructor, I create a named shared memory area unique to my process. In that named shared memory area, I store a pointer to myself, and nothing else. When future singletons get constructed, they reopen the same named shared memory area, realise they are a second or third or fourth instance, and redirect themselves to use the original singleton. They add their own address to the shared memory area. When a shared library containing the original singleton gets dynamically kicked out, it asks the next singleton in the list of singletons in the shared memory area to take over. It transfers all its contents to the new master singleton, and the shared memory list is updated. I appreciate how brutal and overkill this solution is. However, it solves the problem well, in perpetuity, and under whatever crazy weird settings or use cases any user could think of. Niall

Robert Ramey wrote:

I'm quite confident that the problem will be solved in the near future just by doing the work to fix it. It's not rocket science, it just requires investment of time and effort by someone who understands the library and C++

The problem has already been fixed, by Alexander Grund. The fix, in its latest incarnation, is in #131. To recap, - the Serialization library crashes on exit in certain scenarios, as demonstrated by the tests in #111 and #128. - this is caused by the fact that singleton<X>::is_destroyed() always returns false, as demonstrated by the test in #129. (At extended_type_info_typeid.cpp:99, the code tests singleton<tkmap>::is_destroyed(), and if false, proceeds to use singleton<tkmap>::get_mutable_instance(). But since is_destroyed always returns false, the check doesn't work and doesn't prevent the library from accessing the destroyed tkmap in the scenario in question.) - a fix for is_destroyed() is proposed in #131. It makes the tests from #111/#128/#129 pass.

...

The problem has already been fixed, by Alexander Grund. The fix, in its latest incarnation, is in #131.

To recap,

- the Serialization library crashes on exit in certain scenarios, as demonstrated by the tests in #111 and #128.

- this is caused by the fact that singleton<X>::is_destroyed() always returns false, as demonstrated by the test in #129.

(At extended_type_info_typeid.cpp:99, the code tests singleton<tkmap>::is_destroyed(), and if false, proceeds to use singleton<tkmap>::get_mutable_instance(). But since is_destroyed always returns false, the check doesn't work and doesn't prevent the library from accessing the destroyed tkmap in the scenario in question.)

- a fix for is_destroyed() is proposed in #131. It makes the tests from #111/#128/#129 pass.

Hmmm - OK - I'll take yet another look at this.

Robert Ramey

With the upcoming release of 1.69 can we get those PRs finally merged now? What is still missing?

I merged your latest version of your "test" and it failed no where.

Can you point me to the merge or commit and the passing travis build? All I have seen is some of my PR manually put into other commits 65f05dc2099e376f5306cc0c16c29ec0a1a1d2d4 and 237ff81ac62cb9cf8206a050647a1b47601ce4bd and due to not using the full MR it fails to even link on linux: https://travis-ci.org/boostorg/serialization/jobs/446809035 which is probably why you removed it in c5549b2d6ba79a60a98ddceb854954c090c70cb1 Why don't you just merge the open PR which does work as intended and DOES fail on travis?

I also managed to reproduce the remaining test failure - test_dll_exported - on my own machine and have traced it to an related issue which has now been fixed.  I've run all tests on clang and several gcc compilers on my own system.  I'm waiting for final results on the test matrix and appveyor for windows.  These were failing but I need to upload a jamfile tweak. Good to hear another issue is resolved. Sooo - I believe that that there are no pending issues regarding singletons, visibility and export.

Unfortunately not true. 1) The crash-producing test is not or wrongly included 2) The is_destroyed test is not merged and the bug not fixed Again: Please simply merge my PRs (at least the "simple ones" with the tests) or tell me, what you don't understand or like about them so I can change them without breaking them. We are running in circles here where you do not seem to understand my intentions and hence the bugs, change things "based on my changes" but without using the important parts. Others have already confirmed the validity of my tests and that my fix works, so it can't be bad.

The problem has already been fixed, by Alexander Grund. The fix, in its latest incarnation, is in #131.

To recap,

- the Serialization library crashes on exit in certain scenarios, as demonstrated by the tests in #111 and #128.

- this is caused by the fact that singleton<X>::is_destroyed() always returns false, as demonstrated by the test in #129.

(At extended_type_info_typeid.cpp:99, the code tests singleton<tkmap>::is_destroyed(), and if false, proceeds to use singleton<tkmap>::get_mutable_instance(). But since is_destroyed always returns false, the check doesn't work and doesn't prevent the library from accessing the destroyed tkmap in the scenario in question.)

- a fix for is_destroyed() is proposed in #131. It makes the tests from #111/#128/#129 pass.

Thanks for voicing this again. From the comments of you (and a few others) I condensed the fix code, and (tried to) make the tests and fix more easily understandable. As such the code comments in the fix are absolutely essential so future developers won't break it again. These comments and the discussion about it were the reason to bring it up in this way. The tests and fix were already there but not merged for reasons absolutely unclear to me as there was no discussion. This discussion brought some of them up so I could work on them. Thanks again. Minor note: "singleton<X>::is_destroyed() always returns false" is not completely true. There is 1 use case where it does return true: "struct Foo: singleton<Foo>" (as shown in the test). So the correct statement would be "singleton<X>::is_destroyed() always returns false when X does not inherit from singleton<X>". Note that this is the 2nd official use case.

On 10/18/18 10:19 PM, Niall Douglas via Boost wrote:

The way I have solved this problem is brutal, but it works. In my singleton constructor, I create a named shared memory area unique to my process. In that named shared memory area, I store a pointer to myself, and nothing else.

Since you are using shared memory, another (untested) solution could be to let the singleton data be instantiated in shared memory together with a reference count. A singleton is simply a proxy for this data, and the reference count is use to track which singleton should create or destroy the data.