From f95b189fe3fb96f11e308325c37499b8f7e921c5 Mon Sep 17 00:00:00 2001 From: Pablo Arias Date: Sun, 4 Nov 2018 20:12:42 +0100 Subject: [PATCH 1/4] Added a global option in tweakme.h that disabled global registration of loggers. fixes #712 --- include/spdlog/async.h | 5 ++++- include/spdlog/details/registry.h | 10 +--------- include/spdlog/spdlog.h | 5 ++++- include/spdlog/tweakme.h | 8 ++++++++ 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/include/spdlog/async.h b/include/spdlog/async.h index 9264a4e3..bf7dee46 100644 --- a/include/spdlog/async.h +++ b/include/spdlog/async.h @@ -52,7 +52,10 @@ struct async_factory_impl auto sink = std::make_shared(std::forward(args)...); auto new_logger = std::make_shared(std::move(logger_name), std::move(sink), std::move(tp), OverflowPolicy); - registry_inst.register_and_init(new_logger); + registry_inst.init_with_global_defaults(new_logger); +#ifndef SPDLOG_DISABLE_GLOBAL_REGISTRATION + registry_inst.register_logger(new_logger); +#endif return new_logger; } }; diff --git a/include/spdlog/details/registry.h b/include/spdlog/details/registry.h index 439395f7..8f7e7f01 100644 --- a/include/spdlog/details/registry.h +++ b/include/spdlog/details/registry.h @@ -47,13 +47,8 @@ public: loggers_[logger_name] = std::move(new_logger); } - void register_and_init(std::shared_ptr new_logger) + void init_with_global_defaults(std::shared_ptr new_logger) { - std::lock_guard lock(logger_map_mutex_); - auto logger_name = new_logger->name(); - throw_if_exists_(logger_name); - - // set the global formatter pattern new_logger->set_formatter(formatter_->clone()); if (err_handler_) @@ -63,9 +58,6 @@ public: new_logger->set_level(level_); new_logger->flush_on(flush_level_); - - // add to registry - loggers_[logger_name] = std::move(new_logger); } std::shared_ptr get(const std::string &logger_name) diff --git a/include/spdlog/spdlog.h b/include/spdlog/spdlog.h index bcca2e84..d2d8a194 100644 --- a/include/spdlog/spdlog.h +++ b/include/spdlog/spdlog.h @@ -29,7 +29,10 @@ struct synchronous_factory { auto sink = std::make_shared(std::forward(args)...); auto new_logger = std::make_shared(std::move(logger_name), std::move(sink)); - details::registry::instance().register_and_init(new_logger); + details::registry::instance().init_with_global_defaults(new_logger); +#ifndef SPDLOG_DISABLE_GLOBAL_REGISTRATION + details::registry::instance().register_logger(new_logger); +#endif return new_logger; } }; diff --git a/include/spdlog/tweakme.h b/include/spdlog/tweakme.h index d91159f7..a18593c4 100644 --- a/include/spdlog/tweakme.h +++ b/include/spdlog/tweakme.h @@ -128,3 +128,11 @@ // // #define SPDLOG_DISABLE_DEFAULT_LOGGER /////////////////////////////////////////////////////////////////////////////// + +/////////////////////////////////////////////////////////////////////////////// +// Uncomment to disable global logger registration +// This is useful if you don't need loggers to be globally accessible and part of the global registry. +// This option will allow you to use the same name for different loggers. +// +// #define SPDLOG_DISABLE_GLOBAL_REGISTRATION +/////////////////////////////////////////////////////////////////////////////// From fbc58ebef82b6aa95b10f11e9ef2e69df255e022 Mon Sep 17 00:00:00 2001 From: Pablo Arias Date: Sat, 10 Nov 2018 14:34:04 +0100 Subject: [PATCH 2/4] * using API call instead of macro for toggling automatic registration * added unit test for disabling automatic registration --- include/spdlog/async.h | 5 +---- include/spdlog/details/registry.h | 13 ++++++++++++- include/spdlog/spdlog.h | 11 +++++++---- include/spdlog/tweakme.h | 8 -------- tests/test_registry.cpp | 17 +++++++++++++++++ 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/include/spdlog/async.h b/include/spdlog/async.h index bf7dee46..971becd7 100644 --- a/include/spdlog/async.h +++ b/include/spdlog/async.h @@ -52,10 +52,7 @@ struct async_factory_impl auto sink = std::make_shared(std::forward(args)...); auto new_logger = std::make_shared(std::move(logger_name), std::move(sink), std::move(tp), OverflowPolicy); - registry_inst.init_with_global_defaults(new_logger); -#ifndef SPDLOG_DISABLE_GLOBAL_REGISTRATION - registry_inst.register_logger(new_logger); -#endif + registry_inst.initialize_logger(new_logger); return new_logger; } }; diff --git a/include/spdlog/details/registry.h b/include/spdlog/details/registry.h index 8f7e7f01..f6bfe0bd 100644 --- a/include/spdlog/details/registry.h +++ b/include/spdlog/details/registry.h @@ -47,7 +47,7 @@ public: loggers_[logger_name] = std::move(new_logger); } - void init_with_global_defaults(std::shared_ptr new_logger) + void initialize_logger(std::shared_ptr new_logger) { new_logger->set_formatter(formatter_->clone()); @@ -58,6 +58,11 @@ public: new_logger->set_level(level_); new_logger->flush_on(flush_level_); + + if (automatic_registration_) + { + register_logger(new_logger); + } } std::shared_ptr get(const std::string &logger_name) @@ -215,6 +220,11 @@ public: return tp_mutex_; } + void set_automatic_registration(bool automatic_regsistration) + { + automatic_registration_ = automatic_regsistration; + } + static registry &instance() { static registry s_instance; @@ -261,6 +271,7 @@ private: std::shared_ptr tp_; std::unique_ptr periodic_flusher_; std::shared_ptr default_logger_; + bool automatic_registration_ = true; }; } // namespace details diff --git a/include/spdlog/spdlog.h b/include/spdlog/spdlog.h index d2d8a194..8c07012e 100644 --- a/include/spdlog/spdlog.h +++ b/include/spdlog/spdlog.h @@ -29,10 +29,7 @@ struct synchronous_factory { auto sink = std::make_shared(std::forward(args)...); auto new_logger = std::make_shared(std::move(logger_name), std::move(sink)); - details::registry::instance().init_with_global_defaults(new_logger); -#ifndef SPDLOG_DISABLE_GLOBAL_REGISTRATION - details::registry::instance().register_logger(new_logger); -#endif + details::registry::instance().initialize_logger(new_logger); return new_logger; } }; @@ -128,6 +125,12 @@ inline void shutdown() details::registry::instance().shutdown(); } +// Automatic registration of loggers when using spdlog::create() or spdlog::create_async +inline void set_automatic_registration(bool automatic_registation) +{ + details::registry::instance().set_automatic_registration(automatic_registation); +} + // API for using default logger (stdout_color_mt), // e.g: spdlog::info("Message {}", 1); // diff --git a/include/spdlog/tweakme.h b/include/spdlog/tweakme.h index a18593c4..d91159f7 100644 --- a/include/spdlog/tweakme.h +++ b/include/spdlog/tweakme.h @@ -128,11 +128,3 @@ // // #define SPDLOG_DISABLE_DEFAULT_LOGGER /////////////////////////////////////////////////////////////////////////////// - -/////////////////////////////////////////////////////////////////////////////// -// Uncomment to disable global logger registration -// This is useful if you don't need loggers to be globally accessible and part of the global registry. -// This option will allow you to use the same name for different loggers. -// -// #define SPDLOG_DISABLE_GLOBAL_REGISTRATION -/////////////////////////////////////////////////////////////////////////////// diff --git a/tests/test_registry.cpp b/tests/test_registry.cpp index 68833012..57976083 100644 --- a/tests/test_registry.cpp +++ b/tests/test_registry.cpp @@ -93,3 +93,20 @@ TEST_CASE("set_default_logger(nullptr)", "[registry]") spdlog::set_default_logger(nullptr); REQUIRE_FALSE(spdlog::default_logger()); } + +TEST_CASE("disable automatic registration", "[registry]") +{ + // set some global parameters + spdlog::level::level_enum log_level = spdlog::level::level_enum::warn; + spdlog::set_level(log_level); + // but disable automatic registration + spdlog::set_automatic_registration(false); + auto logger1 = spdlog::create(tested_logger_name, "filename", 11, 59); + auto logger2 = spdlog::create_async(tested_logger_name2); + // loggers should not be part of the registry + REQUIRE_FALSE(spdlog::get(tested_logger_name)); + REQUIRE_FALSE(spdlog::get(tested_logger_name2)); + // but make sure they are still initialized according to global defaults + REQUIRE(logger1->level() == log_level); + REQUIRE(logger2->level() == log_level); +} From 10895796b240113416b4bc5bd30002bea8601aaf Mon Sep 17 00:00:00 2001 From: Pablo Arias Date: Sat, 10 Nov 2018 16:55:35 +0100 Subject: [PATCH 3/4] Added mutexes to protect logger initialization and toggling automatic registration --- include/spdlog/details/registry.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/spdlog/details/registry.h b/include/spdlog/details/registry.h index f6bfe0bd..e22b8bbc 100644 --- a/include/spdlog/details/registry.h +++ b/include/spdlog/details/registry.h @@ -49,6 +49,7 @@ public: void initialize_logger(std::shared_ptr new_logger) { + std::lock_guard lock(logger_map_mutex_); new_logger->set_formatter(formatter_->clone()); if (err_handler_) @@ -222,6 +223,7 @@ public: void set_automatic_registration(bool automatic_regsistration) { + std::lock_guard lock(logger_map_mutex_); automatic_registration_ = automatic_regsistration; } From a446f187c19efc22d4d275ae397cdfc62c60ad1e Mon Sep 17 00:00:00 2001 From: Pablo Arias Date: Sat, 10 Nov 2018 17:38:23 +0100 Subject: [PATCH 4/4] Fixed deadlock by recursive mutex --- include/spdlog/details/registry.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/spdlog/details/registry.h b/include/spdlog/details/registry.h index e22b8bbc..af261ce7 100644 --- a/include/spdlog/details/registry.h +++ b/include/spdlog/details/registry.h @@ -62,7 +62,8 @@ public: if (automatic_registration_) { - register_logger(new_logger); + throw_if_exists_(new_logger->name()); + loggers_[new_logger->name()] = std::move(new_logger); } }