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

Missing "shutdown" in the "spdlog::details::registry" destructor #3109

Open
xmirabel opened this issue Jun 12, 2024 · 3 comments
Open

Missing "shutdown" in the "spdlog::details::registry" destructor #3109

xmirabel opened this issue Jun 12, 2024 · 3 comments

Comments

@xmirabel
Copy link

CONTEXT:
I have a specific use case :

  • a generic simulation application executable
  • some models in dynamic libraries following a standard API (called FMI)
  • no link between them at compilation

The simulation is run, then

  • dynamically load each model library
  • call some function
  • can unload some model during execution
  • can reload some model (after debugging them) during the same execution

Each model use its own spdlog multisink logger :

  • to the main console for high level traces (critical, error, warning)
  • to a specific model log file for each model
  • the logger is registered to be able to flush every 2 seconds

Before a model is unloaded, its logger calls spdlog::drop

BUG:

  • when unloading a model: a violent crash happens (the periodic timer seems implies it)
  • if we remove registring and periodic flushing then everything is OK

WORKAROUND FOUND

  • we create an exact full copy of the spdlog::details::registry in our project
  • comment the constructor content (adding of a default logger)
  • add a destructor containing "shutdown" and it works very well.

PERENNIAL SOLUTION
Could you please add a destructor with shutdown in the original spdlog::details::registry?

@tt4g
Copy link
Contributor

tt4g commented Jun 12, 2024

It is not safe to call shutdown() in the spdlog::details::registry destructor, which is a static variable object.
This is because calling shutdown destroys threads used internally, but there is no guarantee that these can be safely executed outside the main() function.
shutdown() should always be called by the developer at the end of the main() function.

It is well known that after the main() function exits, the file descriptors that were opened are cleaned up, but similarly the resources that were associated with threads and mutexes are also cleaned up. It is not known which is called first, this cleanup process or the destructor of the static variable.

The phenomenon of threads and mutex crashing when spdlog is used before main() is called on Windows (i.e DLLMain() #2304 (comment) #2998 (comment)) shows that it is unsafe.

@xmirabel
Copy link
Author

xmirabel commented Jun 14, 2024

I work on Linux Ubuntu 22.04.
Is there also this issue on this operating system?

In my case :

  • the application (main) is first run,
  • then a dynamic load of each library is done (dlopen) and
  • the loggers are created and registered (only to have a period flush) at a main library facade class is instantiated in the library
  • then some libraries are sometimes unloaded

spdlog is not used at all by the main application

@tt4g
Copy link
Contributor

tt4g commented Jun 14, 2024

So why not have a function that cleanup library's resources and call it before the library is unloaded by dlclose()?

It is a common design to provide an initialization function and a cleanup function, which are called before and after the library resource is used (e.g., curl_easy_init()() and curl_easy_cleanup()).

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

No branches or pull requests

2 participants