spdlog: do not drop logger in destructor

For static objects, the order of destruction is in reverse of the order
of construction, but the latter is hard to control except among static
objects defined within the same translation unit. This means that spdlog
may be destructed before spdlog::drop() is invoked in the destructor,
which leads to a segmentation fault due to memory use-after-free.

Move call of spdlog::drop() from destructor to main() function, to
ensure the logger may be re-registered with the same name in tests.
Use Scope Guard pattern to implement RAII for logger registration.

Link: ros2/rclcpp#933
Link: gabime/spdlog#1738
Link: gabime/spdlog#2113
Link: https://en.wikibooks.org/wiki/More_C++_Idioms/Scope_Guard
Closes: https://hsdes.intel.com/appstore/article/#/22019839238
Signed-off-by: Peter Colberg <[email protected]>

libraries/afu-test/afu_test.h

@@ -151,6 +151,27 @@ inline std::vector<std::string> spdlog_levels() {
 inline std::vector<std::string> spdlog_levels() { return SPDLOG_LEVEL_NAMES; }
 #endif // SPDLOG_VERSION
 
+// Dropping a logger is optional, but not doing so will cause an exception
+// when re-registering a logger with the same name, e.g., in test runs.
+class scoped_register_logger {
+public:
+ scoped_register_logger(std::shared_ptr<spdlog::logger> logger)
+ : logger_(std::move(logger))
+ {
+ spdlog::register_logger(logger_);
+ }
+
+ ~scoped_register_logger() {
+ spdlog::drop(logger_->name());
+ }
+
+ scoped_register_logger(scoped_register_logger const &) = delete;
+ void operator=(scoped_register_logger const &) = delete;
+
+private:
+ std::shared_ptr<spdlog::logger> logger_;
+};
+
 class afu {
 public:
  typedef int (*command_fn)(afu *afu, CLI::App *app);
@@ -186,8 +207,6 @@ class afu {
  app_.add_option("-t,--timeout", timeout_msec_, "test timeout (msec)")->default_str(std::to_string(timeout_msec_));
  }
  virtual ~afu() {
- if (logger_)
- spdlog::drop(logger_->name());
  }
 
  CLI::App & cli() { return app_; }
@@ -310,7 +329,7 @@ class afu {
 
  auto console_sink = std::make_shared<spdlog::sinks::stdout_color_sink_mt>();
  logger_ = std::make_shared<spdlog::logger>(test->name(), console_sink);
- spdlog::register_logger(logger_);
+ scoped_register_logger register_logger(logger_);
  logger_->set_level(spdlog::level::from_str(log_level_));
 
  int res = open_handle(test->afu_id());

samples/cxl_hello_fpga/he_cache_test.h

@@ -263,6 +263,27 @@ inline std::vector<std::string> spdlog_levels() {
 inline std::vector<std::string> spdlog_levels() { return SPDLOG_LEVEL_NAMES; }
 #endif // SPDLOG_VERSION
 
+// Dropping a logger is optional, but not doing so will cause an exception
+// when re-registering a logger with the same name, e.g., in test runs.
+class scoped_register_logger {
+public:
+ scoped_register_logger(std::shared_ptr<spdlog::logger> logger)
+ : logger_(std::move(logger))
+ {
+ spdlog::register_logger(logger_);
+ }
+
+ ~scoped_register_logger() {
+ spdlog::drop(logger_->name());
+ }
+
+ scoped_register_logger(scoped_register_logger const &) = delete;
+ void operator=(scoped_register_logger const &) = delete;
+
+private:
+ std::shared_ptr<spdlog::logger> logger_;
+};
+
 class afu {
  public:
  typedef int (*command_fn)(afu *afu, CLI::App *app);
@@ -297,7 +318,6 @@ class afu {
  }
  virtual ~afu() {
  if (fd_ > 0) close(fd_);
- if (logger_) spdlog::drop(logger_->name());
  }
 
  CLI::App &cli() { return app_; }
@@ -449,7 +469,7 @@ class afu {
 
  auto console_sink = std::make_shared<spdlog::sinks::stdout_color_sink_mt>();
  logger_ = std::make_shared<spdlog::logger>(test->name(), console_sink);
- spdlog::register_logger(logger_);
+ scoped_register_logger register_logger(logger_);
  logger_->set_level(spdlog::level::from_str(log_level_));
  current_command_ = test;
  if (find_dev_feature() != 0) {

samples/cxl_host_exerciser/he_cache_test.h

@@ -273,6 +273,27 @@ inline std::vector<std::string> spdlog_levels() {
 inline std::vector<std::string> spdlog_levels() { return SPDLOG_LEVEL_NAMES; }
 #endif // SPDLOG_VERSION
 
+// Dropping a logger is optional, but not doing so will cause an exception
+// when re-registering a logger with the same name, e.g., in test runs.
+class scoped_register_logger {
+public:
+ scoped_register_logger(std::shared_ptr<spdlog::logger> logger)
+ : logger_(std::move(logger))
+ {
+ spdlog::register_logger(logger_);
+ }
+
+ ~scoped_register_logger() {
+ spdlog::drop(logger_->name());
+ }
+
+ scoped_register_logger(scoped_register_logger const &) = delete;
+ void operator=(scoped_register_logger const &) = delete;
+
+private:
+ std::shared_ptr<spdlog::logger> logger_;
+};
+
 class afu {
 public:
  typedef int (*command_fn)(afu *afu, CLI::App *app);
@@ -304,8 +325,6 @@ class afu {
 
  if (fd_ > 0)
  close(fd_);
- if (logger_)
- spdlog::drop(logger_->name());
  }
 
  CLI::App &cli() { return app_; }
@@ -459,7 +478,7 @@ class afu {
 
  auto console_sink = std::make_shared<spdlog::sinks::stdout_color_sink_mt>();
  logger_ = std::make_shared<spdlog::logger>(test->name(), console_sink);
- spdlog::register_logger(logger_);
+ scoped_register_logger register_logger(logger_);
  logger_->set_level(spdlog::level::from_str(log_level_));
  current_command_ = test;
  if (find_dev_feature() != 0) {