From 63016ad9659cf9422624ce1afd7fb7decf06b14b Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Mon, 29 Jul 2024 11:47:04 +0200 Subject: [PATCH] MEM management: Add data storage only destructed after memleak detection. Add a new API to store data that is guaranteed to not be freed before the memleak detector has run. This will be used in next commit by the readfile code to improve reporting on leaks from blendfile readingi process. This is done by a two-layer approach: A new templated `MEM_construct_leak_detection_data` allows to create any type of data. Its ownership and lifetime are handled internally, and guaranteed to not be destroyed before the memleak detector has run. Add a new template-based 'allocation string storage' system to `intern/memutil`. This uses the new `Guardedalloc Persistent Storage` system to store all 'complex' allocation messages, that cannot be defined as literals. Internally, the storage is done through an owning reference (a `shared_ptr`) of the created data into a mutex-protected static vector. `MEM_init_memleak_detection` code ensures that this static storage is created before the memleak detection data, so that it is destructed after the memleak detector has ran. The main container (`AllocStringStorageContainer`) is wrapping a map of `{string -> AllocStringStorage}`. The key is a storage identifier. Each storage is also a map wrapped into a simple templated API class (`AllocStringStorage`), where the values are the alloc strings, and the keys type is defined by the user code. Pull Request: https://projects.blender.org/blender/blender/pulls/125320 --- intern/guardedalloc/MEM_guardedalloc.h | 19 +++ intern/guardedalloc/intern/leak_detector.cc | 16 +++ .../mallocn_intern_function_pointers.hh | 16 +++ intern/memutil/CMakeLists.txt | 6 +- intern/memutil/MEM_alloc_string_storage.hh | 118 ++++++++++++++++++ .../intern/MEM_alloc_string_storage.cc | 21 ++++ 6 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 intern/memutil/MEM_alloc_string_storage.hh create mode 100644 intern/memutil/intern/MEM_alloc_string_storage.cc diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h index 55f8ab926f0..5d6e19f3659 100644 --- a/intern/guardedalloc/MEM_guardedalloc.h +++ b/intern/guardedalloc/MEM_guardedalloc.h @@ -270,6 +270,8 @@ void MEM_use_guarded_allocator(void); #ifdef __cplusplus +# include +# include # include # include # include @@ -421,6 +423,23 @@ template inline T *MEM_cnew(const char *allocation_name, const T &ot */ \ void operator delete(void * /*ptr_to_free*/, void * /*ptr*/) {} +/** + * Construct a T that will only be destructed after leak detection is run. + * + * This call is thread-safe. Calling code should typically keep a reference to that data as a + * `static thread_local` variable, or use some lock, to prevent concurrent accesses. + * + * The returned value should not own any memory allocated with `MEM_*` functions, since these would + * then be detected as leaked. + */ +template T &MEM_construct_leak_detection_data(Args &&...args) +{ + std::shared_ptr data = std::make_shared(std::forward(args)...); + std::any any_data = std::make_any>(data); + mem_guarded::internal::add_memleak_data(any_data); + return *data; +} + #endif /* __cplusplus */ #endif /* __MEM_GUARDEDALLOC_H__ */ diff --git a/intern/guardedalloc/intern/leak_detector.cc b/intern/guardedalloc/intern/leak_detector.cc index f4895d182ca..af478a2b38f 100644 --- a/intern/guardedalloc/intern/leak_detector.cc +++ b/intern/guardedalloc/intern/leak_detector.cc @@ -6,8 +6,11 @@ * \ingroup intern_mem */ +#include #include /* Needed for `printf` on WIN32/APPLE. */ #include +#include +#include #include "MEM_guardedalloc.h" #include "mallocn_intern.hh" @@ -63,6 +66,11 @@ void MEM_init_memleak_detection() /* Calling this ensures that the memory usage counters outlive the memory leak detection. */ memory_usage_init(); + /* Ensure that the static memleak data storage is initialized before the #MemLeakPrinter one, so + * that it outlives the memory leak detection. */ + std::any any_data = std::make_any(0); + mem_guarded::internal::add_memleak_data(any_data); + /** * This variable is constructed when this function is first called. This should happen as soon as * possible when the program starts. @@ -84,3 +92,11 @@ void MEM_enable_fail_on_memleak() { fail_on_memleak = true; } + +void mem_guarded::internal::add_memleak_data(std::any data) +{ + static std::mutex mutex; + static std::vector data_vec; + std::lock_guard lock{mutex}; + data_vec.push_back(std::move(data)); +} diff --git a/intern/guardedalloc/intern/mallocn_intern_function_pointers.hh b/intern/guardedalloc/intern/mallocn_intern_function_pointers.hh index 7c19e43d195..064ded635d0 100644 --- a/intern/guardedalloc/intern/mallocn_intern_function_pointers.hh +++ b/intern/guardedalloc/intern/mallocn_intern_function_pointers.hh @@ -27,4 +27,20 @@ extern void *(*mem_mallocN_aligned_ex)(size_t len, const char *str, AllocationType allocation_type); +/** + * Store a std::any into a static opaque storage vector. The only purpose of this call is to + * control the lifetime of the given data, there is no way to access it from here afterwards. User + * code is expected to keep its own reference to the data contained in the `std::any` as long as it + * needs it. + * + * Typically, this `any` should contain a `shared_ptr` to the actual data, to ensure that the data + * itself is not duplicated, and that the static storage does become an owner of it. + * + * That way, the memleak data does not get destructed before the static storage is. Since this + * storage is created before the memleak detection data (see the implementation of + * #MEM_init_memleak_detection), it is guaranteed to happen after the execution and destruction of + * the memleak detector. + */ +void add_memleak_data(std::any data); + } // namespace mem_guarded::internal diff --git a/intern/memutil/CMakeLists.txt b/intern/memutil/CMakeLists.txt index 1ee3ff1fe34..7ecb96e90e4 100644 --- a/intern/memutil/CMakeLists.txt +++ b/intern/memutil/CMakeLists.txt @@ -3,7 +3,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later set(INC - . + PUBLIC . .. ) @@ -14,16 +14,20 @@ set(INC_SYS set(SRC intern/MEM_CacheLimiterC-Api.cpp intern/MEM_RefCountedC-Api.cpp + intern/MEM_alloc_string_storage.cc MEM_Allocator.h MEM_CacheLimiter.h MEM_CacheLimiterC-Api.h MEM_RefCounted.h MEM_RefCountedC-Api.h + MEM_alloc_string_storage.hh ) set(LIB PRIVATE bf::blenlib + PRIVATE bf::intern::guardedalloc ) blender_add_lib(bf_intern_memutil "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") +add_library(bf::intern::memutil ALIAS bf_intern_memutil) diff --git a/intern/memutil/MEM_alloc_string_storage.hh b/intern/memutil/MEM_alloc_string_storage.hh new file mode 100644 index 00000000000..8ae9a8d2b14 --- /dev/null +++ b/intern/memutil/MEM_alloc_string_storage.hh @@ -0,0 +1,118 @@ +/* SPDX-FileCopyrightText: 2024 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ +#pragma once + +/** \file + * \ingroup intern_memutil + * + * Implement a static storage for complex, non-static allocation strings passed MEM_guardedalloc + * functions. + */ + +#include +#include +#include +#include + +namespace intern::memutil { + +/** + * A 'static' storage of allocation strings, with a simple API to set and retrieve them. + * + * This is a templated wrapper around a std::unordered_map, to allow custom key types. + */ +template typename hashT> class AllocStringStorage { + std::unordered_map> storage_; + + public: + /** + * Check whether the given key exists in the storage. + * + * \return `true` if the \a key is found in storage, false otherwise. + */ + bool contains(keyT &key) + { + return storage_.count(key) != 0; + } + + /** + * Return the alloc string for the given key in the storage. + * + * \return A pointer to the stored string if \a key is found, `nullptr` otherwise. + */ + const char *find(keyT &key) + { + if (storage_.count(key) != 0) { + return storage_[key].c_str(); + } + return nullptr; + } + + /** + * Insert the given alloc string in the storage, at the given key, and return a pointer + * to the stored string. + * + * \param alloc_string: The alloc string to store at \a key. + * \return A pointer to the inserted stored string. + */ + const char *insert(keyT &key, std::string alloc_string) + { +#ifndef NDEBUG + assert(storage_.count(key) == 0); +#endif + return (storage_[key] = std::move(alloc_string)).c_str(); + } +}; + +namespace internal { + +/** + * The main container for all #AllocStringStorage. + */ +class AllocStringStorageContainer { + std::unordered_map storage_; + + public: + /** + * Create if necessary, and return the #AllocStringStorage for the given \a storage_identifier. + * + * The template arguments allow to define the type of key used for the mapping to allocation + * strings. + */ + template typename hashT> + std::any &ensure_storage(const std::string &storage_identifier) + { + if (storage_.count(storage_identifier) == 0) { + AllocStringStorage storage_for_identifier; + return (storage_[storage_identifier] = std::make_any>( + std::move(storage_for_identifier))); + } + return storage_[storage_identifier]; + } +}; + +/** + * Ensure that the static AllocStringStorageContainer is defined and created, and return a + * reference to it. + */ +AllocStringStorageContainer &ensure_storage_container(); + +} // namespace internal + +/** + * Return a reference to the AllocStringStorage static data matching the given \a + * storage_identifier, creating it if needed. + * + * \note The storage is `thread_local` data, so access to it is thread-safe as long as it is not + * shared between threads by the user code. + */ +template typename hashT> +AllocStringStorage &alloc_string_storage_get(const std::string &storage_identifier) +{ + internal::AllocStringStorageContainer &storage_container = internal::ensure_storage_container(); + std::any &storage = storage_container.ensure_storage(storage_identifier); + return std::any_cast &>(storage); +} + +} // namespace intern::memutil diff --git a/intern/memutil/intern/MEM_alloc_string_storage.cc b/intern/memutil/intern/MEM_alloc_string_storage.cc new file mode 100644 index 00000000000..0978e582d75 --- /dev/null +++ b/intern/memutil/intern/MEM_alloc_string_storage.cc @@ -0,0 +1,21 @@ +/* SPDX-FileCopyrightText: 2024 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup intern_memutil + */ + +#include "MEM_alloc_string_storage.hh" +#include "MEM_guardedalloc.h" + +namespace intern::memutil::internal { + +AllocStringStorageContainer &ensure_storage_container() +{ + static thread_local AllocStringStorageContainer &storage = + MEM_construct_leak_detection_data(); + return storage; +} + +} // namespace intern::memutil::internal