Fix logger node teardown before RMW shutdown

moveit_core/utils/src/logger.cpp

@@ -39,6 +39,7 @@
 #include <string>
 #include <rsl/random.hpp>
 #include <fmt/format.h>
+#include "logger_detail.hpp"
 
 namespace moveit
 {
@@ -56,7 +57,21 @@ rclcpp::Logger& getGlobalRootLogger()
     try
     {
       static rclcpp::Node::SharedPtr moveit_node = rclcpp::Node::make_shared(name);
-      return moveit_node->get_logger();
+      static std::shared_ptr<std::mutex> s_mutex = detail::registerNodeResetOnPreShutdown(moveit_node);
+
+      // The pre-shutdown callback registered above can run concurrently on
+      // another thread as soon as it's registered (e.g. if rclcpp::shutdown()
+      // races with this, the very first, call to getGlobalRootLogger()), and
+      // may reset moveit_node to null. Lock the same mutex the callback locks
+      // before reading moveit_node, so the read and the reset can't race.
+      std::lock_guard<std::mutex> lock(*s_mutex);
+      if (moveit_node)
+      {
+        return moveit_node->get_logger();
+      }
+      // Shutdown's pre-shutdown callback already reset the node: fall back to
+      // a plain, non-node logger instead of dereferencing a destroyed node.
+      return rclcpp::get_logger(name);
     }
     catch (const std::exception& ex)
     {
@@ -72,8 +87,20 @@ rclcpp::Logger& getGlobalRootLogger()
 
 void setNodeLoggerName(const std::string& name)
 {
-  static auto node = std::make_shared<rclcpp::Node>("moveit", name);
-  getGlobalRootLogger() = node->get_logger();
+  static rclcpp::Node::SharedPtr s_node = std::make_shared<rclcpp::Node>("moveit", name);
+  static std::shared_ptr<std::mutex> s_mutex = detail::registerNodeResetOnPreShutdown(s_node);
+
+  std::lock_guard<std::mutex> lock(*s_mutex);
+  if (s_node)
+  {
+    getGlobalRootLogger() = s_node->get_logger();
+  }
+  // If the node has already been reset by a pre-shutdown callback from an
+  // earlier rclcpp::shutdown(), leave the global logger untouched rather
+  // than dereferencing a destroyed node. The previously assigned Logger
+  // remains valid: rclcpp::Logger owns its logger-name state independently
+  // and does not retain a reference to the Node, so destroying the Node
+  // does not invalidate the Logger.
 }
 
 rclcpp::Logger getLogger(const std::string& name)

moveit_core/utils/src/logger_detail.hpp

@@ -0,0 +1,104 @@
+/*********************************************************************
+ * Software License Agreement (BSD License)
+ *
+ *  Copyright (c) 2026, PickNik Inc.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions
+ *  are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above
+ *     copyright notice, this list of conditions and the following
+ *     disclaimer in the documentation and/or other materials provided
+ *     with the distribution.
+ *   * Neither the name of PickNik Inc. nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ *  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ *  COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ *  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ *  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ *  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *  POSSIBILITY OF SUCH DAMAGE.
+ *********************************************************************/
+
+// Internal implementation detail of logger.cpp, split out into its own
+// non-installed header purely so test_logger.cpp can test it directly
+// without exposing it through the public moveit/utils/logger.hpp API.
+#pragma once
+
+#include <rclcpp/rclcpp.hpp>
+#include <memory>
+#include <mutex>
+
+namespace moveit
+{
+namespace detail
+{
+
+/// Arranges for `node` -- a caller-owned rclcpp::Node::SharedPtr with static
+/// storage duration -- to be reset from an rclcpp pre-shutdown callback
+/// (which runs *before* rcl_shutdown() tears down the associated RMW
+/// context), instead of being left to run its destructor during static
+/// destruction at process exit. Some RMW implementations (e.g. rmw_zenoh_cpp)
+/// abort the process if RMW calls are made after their own process-wide
+/// static state has already been torn down, which otherwise races against
+/// the unspecified destruction order of unrelated function-local static
+/// objects. See moveit/moveit2#3827.
+///
+/// Returns a mutex the caller must lock before reading or writing `node`
+/// afterwards, so a concurrent caller and pre-shutdown callback can't race
+/// on it.
+///
+/// Two independent, deliberate design choices:
+///
+/// A. Node capture: the callback references the caller-owned `node` slot
+///    itself, rather than strongly capturing the Node. A strong Node
+///    capture could create Context -> pre-shutdown callback -> Node ->
+///    Context: Context strongly owns every pre-shutdown callback registered
+///    on it, and Node (via its NodeBase) strongly owns its
+///    rclcpp::Context::SharedPtr, so a callback holding a shared_ptr to the
+///    Node would close that cycle. A plain reference cannot itself be part
+///    of a shared_ptr reference cycle, so this can't happen.
+///
+/// B. Guard/mutex capture: the callback captures the returned mutex by
+///    *weak_ptr*, not shared_ptr, so that in the "rclcpp::shutdown() is
+///    never called" static-destruction path, the caller-owned mutex --
+///    constructed immediately after `node`, so by the standard's
+///    reverse-order-of-completed-construction rule it is destroyed *before*
+///    `node` -- is already gone by the time `node` itself is destroyed. The
+///    weak_ptr lock then fails if the callback fires while `node` is being
+///    (or has been) destroyed, so the callback never touches the `node`
+///    slot while its own static shared_ptr is itself being torn down,
+///    leaving `node` to be destroyed exactly as it would have been before
+///    this fix.
+inline std::shared_ptr<std::mutex> registerNodeResetOnPreShutdown(rclcpp::Node::SharedPtr& node)
+{
+  auto mutex = std::make_shared<std::mutex>();
+  std::weak_ptr<std::mutex> weak_mutex = mutex;
+  node->get_node_base_interface()->get_context()->add_pre_shutdown_callback([weak_mutex, &node] {
+    std::shared_ptr<std::mutex> locked = weak_mutex.lock();
+    if (!locked)
+    {
+      return;
+    }
+    std::lock_guard<std::mutex> lock(*locked);
+    // Drop the reference so ~rclcpp::Node runs now, while the RMW context is
+    // still alive, instead of racing against it at static destruction time.
+    node.reset();
+  });
+  return mutex;
+}
+
+}  // namespace detail
+}  // namespace moveit

moveit_core/utils/test/CMakeLists.txt

@@ -5,6 +5,26 @@ target_link_libraries(logger_dut rclcpp::rclcpp moveit_utils)
 # Install is needed to for launchtest to execute
 install(TARGETS logger_dut DESTINATION lib/${PROJECT_NAME})
 
+# Unit test for the logger's node lifecycle (regression test for
+# moveit/moveit2#3827). Links against the real moveit_utils library; reaches
+# registerNodeResetOnPreShutdown() via the internal (non-installed)
+# src/logger_detail.hpp shared with logger.cpp, without exposing it through the
+# public moveit/utils/logger.hpp API.
+find_package(ament_cmake_gtest REQUIRED)
+ament_add_gtest(test_logger test_logger.cpp)
+target_link_libraries(test_logger moveit_utils)
+
+# Regression test for getGlobalRootLogger()'s before-rclcpp::init() fallback.
+# Deliberately its own single-test executable/process -- see the file comment in
+# test_logger_before_init.cpp for why this can't be a test case inside
+# test_logger.cpp above.
+ament_add_gtest(
+  test_logger_before_init
+  test_logger_before_init.cpp
+  SKIP_LINKING_MAIN_LIBRARIES
+)
+target_link_libraries(test_logger_before_init moveit_utils)
+
 find_package(launch_testing_ament_cmake)
 
 # These tests do not work on Humble as /rosout logging from child loggers does