This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 11e99cb679 Fix DbgCtl use-after-free shutdown crash via leaky
singleton (#12777)
11e99cb679 is described below
commit 11e99cb6796bd5eae3fd6be0822e7f0ec9f86bd0
Author: Brian Neradt <[email protected]>
AuthorDate: Tue Jan 6 14:16:12 2026 -0600
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
---
ci/asan_leak_suppression/regression.txt | 3 ++
include/tsutil/DbgCtl.h | 12 ++---
plugins/esi/test/CMakeLists.txt | 5 +++
plugins/esi/test/esi_test_leak_suppression.txt | 5 +++
plugins/esi/test/print_funcs.cc | 16 +------
src/tsutil/DbgCtl.cc | 62 +++-----------------------
6 files changed, 24 insertions(+), 79 deletions(-)
diff --git a/ci/asan_leak_suppression/regression.txt
b/ci/asan_leak_suppression/regression.txt
index 4e98783f67..b3dfcf8363 100644
--- a/ci/asan_leak_suppression/regression.txt
+++ b/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
diff --git a/include/tsutil/DbgCtl.h b/include/tsutil/DbgCtl.h
index dee8d1ebfb..76cde235f1 100644
--- a/include/tsutil/DbgCtl.h
+++ b/include/tsutil/DbgCtl.h
@@ -67,12 +67,10 @@ public:
//
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 @@ private:
static const _TagData *_new_reference(char const *tag);
- static void _rm_reference();
-
class _RegistryAccessor;
static std::atomic<int> _config_mode;
diff --git a/plugins/esi/test/CMakeLists.txt b/plugins/esi/test/CMakeLists.txt
index 1aff571bcb..c9bc8c2d5f 100644
--- a/plugins/esi/test/CMakeLists.txt
+++ b/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)
diff --git a/plugins/esi/test/esi_test_leak_suppression.txt
b/plugins/esi/test/esi_test_leak_suppression.txt
new file mode 100644
index 0000000000..8a857cbeab
--- /dev/null
+++ b/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
+
diff --git a/plugins/esi/test/print_funcs.cc b/plugins/esi/test/print_funcs.cc
index abc9894761..2596a46205 100644
--- a/plugins/esi/test/print_funcs.cc
+++ b/plugins/esi/test/print_funcs.cc
@@ -63,14 +63,12 @@ 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};
@@ -78,11 +76,9 @@ 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;
@@ -90,16 +86,6 @@ DbgCtl::_new_reference(char const *tag)
return &(*it);
}
-void
-DbgCtl::_rm_reference()
-{
- if (!--_RegistryAccessor::ref_count) {
- for (auto &elem : _RegistryAccessor::registry()) {
- delete[] elem.first;
- }
- }
-}
-
bool
DbgCtl::_override_global_on()
{
diff --git a/src/tsutil/DbgCtl.cc b/src/tsutil/DbgCtl.cc
index 0263c31139..cc78dea8b3 100644
--- a/src/tsutil/DbgCtl.cc
+++ b/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 @@ public:
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,8 +105,8 @@ public:
}
}
- // 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()
@@ -124,20 +114,6 @@ public:
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);
