Fix DbgCtl use-after-free shutdown crash via leaky singleton (#12777)

The DbgCtl registry was experiencing use-after-free crashes during program
shutdown, particularly visible in CI regression tests. The root cause
was the undefined destruction order of static objects in C++.

Problem:
- DbgCtl objects can be static or thread-local with lifetimes spanning program
  execution
- When one compilation unit's DbgCtl destructs and triggers registry cleanup,
  other compilation units may still have DbgCtl objects with pointers into
  that registry
- Thread exit order is also unpredictable, causing similar issues when
  thread-local DbgCtl objects destruct

This implements the "leaky singleton" pattern where the registry is:
- Created on first use
- Never destroyed (destructor is now = default)
- Memory (~20KB) reclaimed by OS at process exit

Fixes: #12776

Author: bneradt

ci/asan_leak_suppression/regression.txt

@@ -19,5 +19,8 @@ leak:RegressionTest_HostDBProcessor
 leak:RegressionTest_DNS
 leak:RegressionTest_UDPNet_echo
 leak:HttpConfig::startup
+# DbgCtl leaky singleton pattern - intentional, see issue #12776.
+leak:DbgCtl::_new_reference
+leak:_new_reference
 # libswoc
 leak:MemArena::make_block

include/tsutil/DbgCtl.h

@@ -67,12 +67,10 @@ class DbgCtl
   //
   DbgCtl() : _ptr{&_No_tag_dummy()} {}
 
-  ~DbgCtl()
-  {
-    if (_ptr != &_No_tag_dummy()) {
-      _rm_reference();
-    }
-  }
+  // Default destructor: the registry uses a "leaky singleton" pattern to avoid
+  // use-after-free crashes during shutdown due to undefined static destruction
+  // order. See issue #12776.
+  ~DbgCtl() = default;
 
   // No copying. Only moving from a tagged to a tagless instance allowed.
   //
@@ -153,8 +151,6 @@ class DbgCtl
 
   static const _TagData *_new_reference(char const *tag);
 
-  static void _rm_reference();
-
   class _RegistryAccessor;
 
   static std::atomic<int> _config_mode;

plugins/esi/test/CMakeLists.txt

@@ -22,6 +22,11 @@ macro(ADD_ESI_TEST NAME)
   add_executable(${NAME} print_funcs.cc ${ARGN})
   target_link_libraries(${NAME} PRIVATE esitest)
   add_catch2_test(NAME ${NAME}_esi COMMAND $<TARGET_FILE:${NAME}>)
+  # Use ASan leak suppression for intentional DbgCtl leaks (issue #12776)
+  set_tests_properties(
+    ${NAME}_esi PROPERTIES ENVIRONMENT
+                           "LSAN_OPTIONS=suppressions=${CMAKE_CURRENT_SOURCE_DIR}/esi_test_leak_suppression.txt"
+  )
 endmacro()
 
 add_esi_test(test_docnode docnode_test.cc)

plugins/esi/test/esi_test_leak_suppression.txt

@@ -0,0 +1,5 @@
+# ESI unit test leak suppressions
+# DbgCtl leaky singleton pattern - intentional, see issue #12776
+leak:DbgCtl::_new_reference
+leak:_new_reference
+

plugins/esi/test/print_funcs.cc

@@ -63,43 +63,29 @@ class DbgCtl::_RegistryAccessor
 {
 public:
   // No mutex protection, assuming unit test is single threaded.
-  //
   static std::map<char const *, bool> &
   registry()
   {
     static std::map<char const *, bool> r;
     return r;
   }
-  static inline int ref_count{0};
 };
 
 std::atomic<int> DbgCtl::_config_mode{1};
 
 DbgCtl::_TagData const *
 DbgCtl::_new_reference(char const *tag)
 {
-  ++_RegistryAccessor::ref_count;
-
   auto it{_RegistryAccessor::registry().find(tag)};
   if (it == _RegistryAccessor::registry().end()) {
-    char *s = new char[std::strlen(tag) + 1];
+    char *s = new char[std::strlen(tag) + 1]; // Never deleted - leaky singleton pattern.
     std::strcpy(s, tag);
     auto r{_RegistryAccessor::registry().emplace(s, true)}; // Tag is always enabled.
     it = r.first;
   }
   return &(*it);
 }
 
-void
-DbgCtl::_rm_reference()
-{
-  if (!--_RegistryAccessor::ref_count) {
-    for (auto &elem : _RegistryAccessor::registry()) {
-      delete[] elem.first;
-    }
-  }
-}
-
 bool
 DbgCtl::_override_global_on()
 {

src/tsutil/DbgCtl.cc

@@ -54,9 +54,6 @@ DbgCtl::operator=(DbgCtl &&src)
   return *this;
 }
 
-// The resistry of fast debug controllers has a ugly implementation to handle the whole-program initialization
-// and destruction order problem with C++.
-//
 class DbgCtl::_RegistryAccessor
 {
 private:
@@ -79,15 +76,8 @@ class DbgCtl::_RegistryAccessor
   private:
     Registry() = default;
 
-    // Mutex must be locked before this is called.
-    //
-    ~Registry()
-    {
-      for (auto &elem : map) {
-        delete[] elem.first;
-      }
-      _mtx.unlock();
-    }
+    // Destructor never called - this is a leaky singleton. See issue #12776.
+    ~Registry() = default;
 
     std::mutex _mtx;
 
@@ -115,29 +105,15 @@ class DbgCtl::_RegistryAccessor
     }
   }
 
-  // This is not static so it can't be called with the registry mutex is unlocked.  It should not be called
-  // after registry is deleted.
+  // This is not static so it can't be called with the registry mutex unlocked.
+  // The registry is never deleted (leaky singleton pattern), so it's always safe to call after creation.
   //
   Registry &
   data()
   {
     return *_registry_instance;
   }
 
-  void
-  delete_registry()
-  {
-    auto r = _registry_instance.load();
-    debug_assert(r != nullptr);
-    _registry_instance = nullptr;
-    delete r;
-    _mtx_is_locked = false;
-  }
-
-  // Reference count of references to Registry.
-  //
-  inline static std::atomic<unsigned> registry_reference_count{0};
-
 private:
   bool _mtx_is_locked{false};
 
@@ -152,13 +128,6 @@ DbgCtl::_new_reference(char const *tag)
 
   _TagData *new_tag_data{nullptr};
 
-  // DbgCtl instances may be declared as static objects in the destructors of objects not destroyed till program exit.
-  // So, we must handle the case where the construction of such instances of DbgCtl overlaps with the destruction of
-  // other instances of DbgCtl.  That is why it is important to make sure the reference count is non-zero before
-  // constructing _RegistryAccessor.  The _RegistryAccessor constructor is thereby able to assume that, if it creates
-  // the Registry, the new Registry will not be destroyed before the mutex in the new Registry is locked.
-
-  ++_RegistryAccessor::registry_reference_count;
   {
     _RegistryAccessor ra;
 
@@ -172,7 +141,7 @@ DbgCtl::_new_reference(char const *tag)
 
     debug_assert(sz > 0);
 
-    char *t = new char[sz + 1]; // Deleted by ~Registry().
+    char *t = new char[sz + 1]; // Never deleted (leaky singleton) - reclaimed by OS at process exit.
     std::memcpy(t, tag, sz + 1);
     _TagData new_elem{t, false};
 
@@ -201,30 +170,11 @@ DbgCtl::_new_reference(char const *tag)
   return new_tag_data;
 }
 
-void
-DbgCtl::_rm_reference()
-{
-  _RegistryAccessor ra;
-
-  debug_assert(ra.registry_reference_count != 0);
-
-  --ra.registry_reference_count;
-
-  if (0 == ra.registry_reference_count) {
-    ra.delete_registry();
-  }
-}
-
 void
 DbgCtl::update(const std::function<bool(const char *)> &f)
 {
   _RegistryAccessor ra;
-
-  if (!ra.registry_reference_count) {
-    return;
-  }
-
-  auto &d{ra.data()};
+  auto             &d{ra.data()};
 
   for (auto &i : d.map) {
     i.second = f(i.first);