From ee7e642592007e17fb311e4960caf2e51e4595b4 Mon Sep 17 00:00:00 2001 From: Dirk Thomas Date: Thu, 4 Apr 2019 09:31:59 -0700 Subject: [PATCH] refactor SignalHandler logger to avoid race during destruction (#682) Signed-off-by: Dirk Thomas --- rclcpp/src/rclcpp/signal_handler.cpp | 36 +++++++++++++++++++++------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/rclcpp/src/rclcpp/signal_handler.cpp b/rclcpp/src/rclcpp/signal_handler.cpp index debad74..ab981be 100644 --- a/rclcpp/src/rclcpp/signal_handler.cpp +++ b/rclcpp/src/rclcpp/signal_handler.cpp @@ -46,18 +46,38 @@ dispatch_semaphore_t SignalHandler::signal_handler_sem_; sem_t SignalHandler::signal_handler_sem_; #endif -SignalHandler & -SignalHandler::get_global_signal_handler() -{ - static SignalHandler signal_handler; - return signal_handler; -} +// The logger must be initialized before the local static variable signal_handler, +// from the method get_global_signal_handler(), so that it is destructed after +// it, because the destructor of SignalHandler uses this logger object. +static rclcpp::Logger g_logger = rclcpp::get_logger("rclcpp"); rclcpp::Logger & SignalHandler::get_logger() { - static rclcpp::Logger logger = rclcpp::get_logger("rclcpp"); - return logger; + return g_logger; +} + +SignalHandler & +SignalHandler::get_global_signal_handler() +{ + // This is initialized after the g_logger static global, ensuring + // SignalHandler::get_logger() may be called from the destructor of + // SignalHandler, according to this: + // + // Variables declared at block scope with the specifier static have static + // storage duration but are initialized the first time control passes + // through their declaration (unless their initialization is zero- or + // constant-initialization, which can be performed before the block is + // first entered). On all further calls, the declaration is skipped. + // + // -- https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables + // + // Which is guaranteed to occur after static initialization for global (see: + // https://en.cppreference.com/w/cpp/language/initialization#Static_initialization), + // which is when g_logger will be initialized. + // And destruction will occur in the reverse order. + static SignalHandler signal_handler; + return signal_handler; } bool