Fix memory leak in TracyVector for non-trivially-destructible types
Summary
This PR fixes a memory leak in TracyVector where element destructors are not called for non-trivially-destructible types. The bug affects production code using nested containers like Vector<Vector<short_ptr<ZoneEvent>>>.
Root Cause
TracyVector is designed as a lightweight container optimized for POD types. However, it's also used with non-trivially-destructible types throughout Tracy's codebase. The current implementation never calls element destructors, causing memory leaks when elements manage resources (e.g., heap-allocated memory in nested vectors).
Solution
Add compile-time type checking using C++17 if constexpr with std::is_trivially_destructible_v<T>:
~Vector() {
if (m_ptr) {
// Smart destruction - call destructors for non-trivial types
// For trivial types, if constexpr is optimized away by compiler, zero overhead
if constexpr (!std::is_trivially_destructible_v<T>) {
for (uint32_t i = 0; i < m_size; i++) {
m_ptr[i].~T();
}
}
free(m_ptr);
}
}
This approach:
- ✅ Zero overhead for trivial types -
if constexpris evaluated at compile time - ✅ Proper cleanup for non-trivial types - destructors are called when needed
- ✅ Backward compatible - doesn't change behavior for existing POD usage
- ✅ Applied to 4 locations: destructor, move assignment,
clear(),Realloc()
Impact on Production Code
This bug affects critical data structures in TracyWorker.hpp:
// Line ~500
Vector<Vector<short_ptr<ZoneEvent>>> zoneChildren;
// Line ~520
Vector<Vector<short_ptr<GpuEvent>>> gpuChildren;
// Line ~540
Vector<Vector<GhostZone>> ghostChildren;
// Line ~560
Vector<Vector<short_ptr<ZoneEvent>>> zoneVectorCache;
TracyVector Usage Statistics:
- 25+ uses in
TracyEvent.hpp(mostly POD types) - 4 critical nested container instances in
TracyWorker.hpp(affected by this bug)
Verification
Tested with AddressSanitizer/LeakSanitizer using Tracy's actual production types:
Before Fix
=================================================================
==12345==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 24 byte(s) in 3 allocation(s) from:
#0 0x7f8b4c in malloc
#1 0x401234 in Vector<Vector<short_ptr<ZoneEvent>>>::push_back
Direct leak of 12 byte(s) in 3 allocation(s) from:
#0 0x7f8b4c in malloc
#1 0x401567 in Vector<short_ptr<ZoneEvent>>::push_back
SUMMARY: AddressSanitizer: 36 byte(s) leaked in 6 allocation(s).
After Fix
=================================================================
==12346==LeakSanitizer has found no leaks.
SUMMARY: AddressSanitizer: 0 byte(s) leaked in 0 allocation(s).
Test Coverage
The fix includes a comprehensive test suite (test_before_after_fix.cpp) with 6 test cases:
- Nested vectors (
Vector<Vector<short_ptr<ZoneEvent>>>) - Move assignment for nested vectors
-
clear()with non-trivial types - POD types (zero overhead verification)
- Complex nested structures (
Vector<Vector<GhostZone>>) - Memory reallocation scenarios
All tests pass with AddressSanitizer validation.
Backward Compatibility
- ✅ No breaking changes
- ✅ Existing POD usage unchanged (compile-time optimization)
- ✅ No performance regression for trivial types
- ✅ Fixes undefined behavior for non-trivial types
Files Changed
-
server/TracyVector.hpp: Added<type_traits>include andif constexprchecks in 4 locations
Request for Feedback
I'd appreciate feedback on:
- Whether this approach aligns with Tracy's design philosophy
- Any additional test cases you'd like to see
- Performance implications (though compile-time checks should have zero overhead)
Thank you for maintaining this excellent profiling tool!
While this is a valid issue (there are indeed leaks caused by tracy::Vector), the implementation of this PR (obviously made by an AI) is incomplete.
For example in Vector::insert we use memmove which is not ok if the object is not trivially movable.
In general, it seems Vector should either be restricted to trivially destructible & trivially movable types or changed to fully support them.
As far as I'm aware, the primary interest of using tracy::Vector over std::vector (also used extensively in the codebase) is its "magic" support used when deserializing and the fact it is small (11bytes + 1byte alignment) and saves memory when there are many.
I would personally just add static asserts to detect types that may cause leaks/issues, replace the currently problematic uses by std::vector (they do not benefit much from using Vector, it's fine if we have std::vector<Vector<short_ptr*>> to keep the low memory overhead), but I'll let @wolfpld weigh in on such things.