From 062936567ede9102fe560cc04b0871fa8cfa5aaa Mon Sep 17 00:00:00 2001 From: bneradt Date: Wed, 7 Jan 2026 17:29:48 -0600 Subject: [PATCH] Fix DenseThreadId static destruction order fiasco The _id_stack vector was being destroyed before thread-local _Id objects, causing use-after-free when _Id::~_Id() tried to write to the freed vector. This patch addresses this by using std::array instead of std::vector. Since std::array has a trivial destructor, the memory remains valid to the threads trying to use it regardless of destruction order. This bug was latent and only exposed when linking order changed (e.g., when adding new source files that changed static initialization order). Here's the valgrind output diagnosing the issue being fixed here: ==29149== Invalid write of size 8 ==29149== at 0x565832: DenseThreadId::_Id::~_Id() ==29149== by 0x6126DD8: ??? (in /usr/lib64/libstdc++.so.6.0.19) ==29149== by 0x6B40059: __cxa_finalize (in /usr/lib64/libc-2.17.so) ==29149== ... ==29149== Address 0xd4be1c0 is 0 bytes inside a block of size 2,048 free'd ==29149== at 0x4C2B6DF: operator delete(void*, unsigned long) ==29149== by 0x6B40059: __cxa_finalize (in /usr/lib64/libc-2.17.so) ==29149== ... ==29149== Block was alloc'd at ==29149== at 0x4C2A593: operator new(unsigned long) ==29149== by 0x566820: std::vector<...>::_M_default_append(unsigned long) ==29149== by 0x56515C: __tls_init --- include/tsutil/DenseThreadId.h | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/include/tsutil/DenseThreadId.h b/include/tsutil/DenseThreadId.h index 17a81ebd56a..5d3f9d38e50 100644 --- a/include/tsutil/DenseThreadId.h +++ b/include/tsutil/DenseThreadId.h @@ -27,21 +27,19 @@ #include "tsutil/Assert.h" +#include #include #include -#include -// Provide an alternate thread id, suitible for use as an array index. +// Provide an alternate thread id, suitable for use as an array index. // class DenseThreadId { public: - // This can onlhy be called during single-threaded initialization. - // - static void - set_num_possible_values(std::size_t num_possible_values) + static constexpr std::size_t + num_possible_values() { - _num_possible_values = num_possible_values; + return _num_possible_values; } static std::size_t @@ -50,23 +48,15 @@ class DenseThreadId return _id.val; } - static std::size_t - num_possible_values() - { - return _num_possible_values; - } - private: - inline static std::mutex _mtx; - inline static std::vector _id_stack; - inline static std::size_t _stack_top_idx; - inline static std::size_t _num_possible_values{256}; + static constexpr std::size_t _num_possible_values{256}; + inline static std::mutex _mtx; + inline static std::array _id_stack; + inline static std::size_t _stack_top_idx; static void _init() { - _id_stack.resize(_num_possible_values); - _stack_top_idx = 0; for (std::size_t i{0}; i < _num_possible_values; ++i) { _id_stack[i] = i + 1; @@ -82,8 +72,8 @@ class DenseThreadId _init(); _inited = true; } - if (_id_stack.size() == _stack_top_idx) { - fatal_error("DenseThreadId: number of threads exceeded maximum {}", unsigned(_id_stack.size())); + if (_num_possible_values == _stack_top_idx) { + fatal_error("DenseThreadId: number of threads exceeded maximum {}", unsigned(_num_possible_values)); } val = _stack_top_idx; _stack_top_idx = _id_stack[_stack_top_idx];