cpp_client_telemetry icon indicating copy to clipboard operation
cpp_client_telemetry copied to clipboard

Decide what to do if LogManager::Initialize is called again with a different token without proper FlushAndTeardown in between

Open maxgolov opened this issue 5 years ago • 3 comments

How do we handle the following user-error (invalid API usage) scenario:

  • customer calls Initialize(Token1)
  • customer forgets to teardown
  • customer calls Initialize(Token2) without properly cleaning up the 1st session

For now the SDK is rather forgiving and handles the 2nd initialize as GetLogger(Token2). That way user apps don't crash, even if some other module calls into Initialize. The proper better way of handling this is to just crash the app, if it attempts doing something like this. IF a module is doing 2nd initialize after the main app did the initialize, then the module should rather be using our Host-Guest approach: either allocating its own logger or anchoring to the main host app logger instead. Basically the module must have its own object instance, not using the main app's singleton.

maxgolov avatar Sep 16 '20 19:09 maxgolov

Why not explicitly allow Initialize() to allow it to do the "logger for Token2" dance? Also also, I would be just ever so delighted if the exciting pointers and instances, singleton or otherwise, had proper lifetime/ownership semantics to make FlushAndTeardown() a bit less likely to explode the world.

larvacea avatar Sep 22 '20 18:09 larvacea

We already allow it. But this may be a bit problematic if Module A (legit owner) vs. Module B (2nd initialize), pretty much unaware of each other - both attempt to perform FlushAndTeardown . We run into non-deterministic behavior, depending on who decides to teardown first. I know we started allowing leaky loggers, so the entire system is now rather forgiving.. But this may present a challenge also, for example, in case if both use global semantic contexts, and Module A vs. Module B are gonna be both polluting each other semantic context with their own variables.

I think, ideally, we should at least trigger some warning, like you are not supposed to be doing this! or otherwise indicate to the caller that they are initializing when the system is already initialized by someone else.

maxgolov avatar Sep 22 '20 18:09 maxgolov

This is also gonna be causing an issue with invalid (old, stale) path to DB, if 2nd caller decides to change the DB path.

maxgolov avatar Oct 22 '20 02:10 maxgolov