Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guarantee Singleton Order Of Destruction #1620

Merged
merged 15 commits into from
May 10, 2022

Conversation

Klaim
Copy link
Member

@Klaim Klaim commented Apr 12, 2022

In C++ namespace-scope static object are created and destructed in an unspecified order before and after main() execution, respectively. The only guarantee is that such objects in the same translation unit shall be constructed in apparition order in the file and destructed in the reverse order. Another guarantee is that local (inside functoins) static object are guaranteed to be created only at the first call to that function, but then there is no guarantee on the destruction order.

This is fine until these objects interacts, that is, at least one of these objects have a dependency/usage of another object defined in a different translation unit.

We currently have singletons which are implemented as global statics spread through various translation units, and several of these singletons are also accessed through construction and destruction of other singletons. Basically, we have undefined behavior around their creation and destruction, so it is not always visible because often memory looks like valid objects even if it is not. Some recent fixes made this even more visible (which is helpful).

The proposed changings tries to eliminate these issues by:

  • using the local static guarantee for creating singletons : they are created on use, which means a singleton needing another singleton should work as long as there is no circular dependency;
  • using the translation-unit order of destruction guarantee: we gathered all singleton object definitoins in one translation unit to guarantee their order of destruction (even if the order of creation doesnt match);
  • added some detection of access to destroyed singletons to reveal future issues;
  • fix issues revealed by this setup (including some circular dependencies).

@Klaim Klaim force-pushed the klaim/fix-singleton branch from e806954 to d4191e5 Compare April 14, 2022 15:53
@Klaim Klaim force-pushed the klaim/fix-singleton branch 2 times, most recently from d283f63 to 07ee63c Compare April 28, 2022 10:04
libmamba/src/core/url.cpp Outdated Show resolved Hide resolved
@Klaim Klaim changed the title Klaim/fix singleton Guarantee Singleton Order Of Destruction May 5, 2022
@Klaim Klaim force-pushed the klaim/fix-singleton branch 3 times, most recently from f3ec7ba to 4e5eaae Compare May 9, 2022 10:32
@Klaim Klaim marked this pull request as ready for review May 9, 2022 12:27
@Klaim Klaim force-pushed the klaim/fix-singleton branch from 4e5eaae to 4be8583 Compare May 9, 2022 13:05
libmamba/include/mamba/api/configuration.hpp Show resolved Hide resolved
libmamba/include/mamba/core/channel_builder.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/context.hpp Show resolved Hide resolved
libmamba/include/mamba/core/error_handling.hpp Outdated Show resolved Hide resolved
libmamba/include/mamba/core/output.hpp Show resolved Hide resolved
libmamba/include/mamba/core/validate.hpp Show resolved Hide resolved
Klaim and others added 15 commits May 10, 2022 12:26
In C++ namespace-scope static object are created and destructed in an
unspecified order before and after `main()` execution, respectively.
The only guarantee is that such objects in the same translation unit
shall be constructed in apparition order in the file and destructed in
the reverse order. Another guarantee is that local (inside functoins)
static object are guaranteed to be created only at the first call to
that function, but then there is no guarantee on the destruction order.

This is fine until these objects interacts, that is, at least one of
these objects have a dependency/usage of another object defined in a
different translation unit.

We currently have singletons which are implemented as global statics
spread through various translation units, and several of these
singletons are also accessed through construction and destruction of
other singletons. Basically, we have undefined behavior around their
creation and destruction, so it is not always visible because often
memory looks like valid objects even if it is not. Some recent fixes
made this even more visible (which is helpful).

This change moves all the singleton object definitions into the same
translation unit to begin improving guarantees on their construction
and destruction order after `main()` execution.
Currently this can only happen when a singleton is used but
it was destroyed.
mamba uses its own (or conda's) json output so libmamba's json output
must be cancelled to avoid outputing json twice (and probably
different)
@Klaim Klaim force-pushed the klaim/fix-singleton branch from 83e04e4 to 141158a Compare May 10, 2022 10:44
@Klaim
Copy link
Member Author

Klaim commented May 10, 2022

I just rebased over master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants