From 236ca8fbe84575504f6ae80aec63c8059b875ef0 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Fri, 24 Jul 2020 12:26:11 +0200 Subject: Allocator: make leak detection work with static variables When definining static variables that own memory, you should use the "construct on first use" idiom. Otherwise, you'll get a warning when Blender exits. More details are provided in D8354. Differential Revision: https://developer.blender.org/D8354 --- intern/guardedalloc/CMakeLists.txt | 1 + intern/guardedalloc/MEM_guardedalloc.h | 4 ++ intern/guardedalloc/intern/leak_detector.cc | 61 ++++++++++++++++++++++ intern/guardedalloc/intern/mallocn_guarded_impl.c | 4 ++ intern/guardedalloc/intern/mallocn_inline.h | 8 +++ intern/guardedalloc/intern/mallocn_intern.h | 11 ++++ intern/guardedalloc/intern/mallocn_lockfree_impl.c | 4 ++ source/blender/makesdna/intern/CMakeLists.txt | 1 + source/blender/makesrna/intern/CMakeLists.txt | 1 + source/blender/makesrna/intern/makesrna.c | 12 ++--- source/blender/windowmanager/intern/wm_init_exit.c | 7 --- source/creator/creator.c | 1 + .../blenloader/blendfile_loading_base_test.cc | 8 --- tests/gtests/testing/CMakeLists.txt | 2 + tests/gtests/testing/testing_main.cc | 3 ++ 15 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 intern/guardedalloc/intern/leak_detector.cc diff --git a/intern/guardedalloc/CMakeLists.txt b/intern/guardedalloc/CMakeLists.txt index cb24df65ba0..1ab365a376a 100644 --- a/intern/guardedalloc/CMakeLists.txt +++ b/intern/guardedalloc/CMakeLists.txt @@ -28,6 +28,7 @@ set(INC_SYS ) set(SRC + ./intern/leak_detector.cc ./intern/mallocn.c ./intern/mallocn_guarded_impl.c ./intern/mallocn_lockfree_impl.c diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h index 1318aa10697..604330bd1d3 100644 --- a/intern/guardedalloc/MEM_guardedalloc.h +++ b/intern/guardedalloc/MEM_guardedalloc.h @@ -211,6 +211,10 @@ extern size_t (*MEM_get_peak_memory)(void) ATTR_WARN_UNUSED_RESULT; extern const char *(*MEM_name_ptr)(void *vmemh); #endif +/** This should be called as early as possible in the program. When it has been called, information + * about memory leaks will be printed on exit. */ +void MEM_initialize_memleak_detection(void); + /* Switch allocator to slower but fully guarded mode. */ void MEM_use_guarded_allocator(void); diff --git a/intern/guardedalloc/intern/leak_detector.cc b/intern/guardedalloc/intern/leak_detector.cc new file mode 100644 index 00000000000..4b2689ee28c --- /dev/null +++ b/intern/guardedalloc/intern/leak_detector.cc @@ -0,0 +1,61 @@ +/* + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +/** \file + * \ingroup MEM + */ + +#include "MEM_guardedalloc.h" +#include "mallocn_intern.h" + +bool leak_detector_has_run = false; +char free_after_leak_detection_message[] = + "Freeing memory after the leak detector has run. This can happen when using " + "static variables in C++ that are defined outside of functions. To fix this " + "error, use the 'construct on first use' idiom."; + +namespace { +class MemLeakPrinter { + public: + ~MemLeakPrinter() + { + leak_detector_has_run = true; + const uint leaked_blocks = MEM_get_memory_blocks_in_use(); + if (leaked_blocks == 0) { + return; + } + const size_t mem_in_use = MEM_get_memory_in_use(); + printf("Error: Not freed memory blocks: %u, total unfreed memory %f MB\n", + leaked_blocks, + (double)mem_in_use / 1024 / 1024); + MEM_printmemlist(); + } +}; +} // namespace + +void MEM_initialize_memleak_detection(void) +{ + /** + * This variable is constructed when this function is first called. This should happen as soon as + * possible when the program starts. + * + * It is destructed when the program exits. During destruction, it will print information about + * leaked memory blocks. Static variables are destructed in reversed order of their + * construction. Therefore, all static variables that own memory have to be constructed after + * this function has been called. + */ + static MemLeakPrinter printer; +} diff --git a/intern/guardedalloc/intern/mallocn_guarded_impl.c b/intern/guardedalloc/intern/mallocn_guarded_impl.c index 5e523204020..2c207935e43 100644 --- a/intern/guardedalloc/intern/mallocn_guarded_impl.c +++ b/intern/guardedalloc/intern/mallocn_guarded_impl.c @@ -898,6 +898,10 @@ void MEM_guarded_freeN(void *vmemh) memt = (MemTail *)(((char *)memh) + sizeof(MemHead) + memh->len); if (memt->tag3 == MEMTAG3) { + if (leak_detector_has_run) { + MemorY_ErroR(memh->name, free_after_leak_detection_message); + } + memh->tag1 = MEMFREE; memh->tag2 = MEMFREE; memt->tag3 = MEMFREE; diff --git a/intern/guardedalloc/intern/mallocn_inline.h b/intern/guardedalloc/intern/mallocn_inline.h index f8bb7861fc9..4e73eb9bad6 100644 --- a/intern/guardedalloc/intern/mallocn_inline.h +++ b/intern/guardedalloc/intern/mallocn_inline.h @@ -33,6 +33,10 @@ #ifndef __MALLOCN_INLINE_H__ #define __MALLOCN_INLINE_H__ +#ifdef __cplusplus +extern "C" { +#endif + MEM_INLINE bool MEM_size_safe_multiply(size_t a, size_t b, size_t *result) { /* A size_t with its high-half bits all set to 1. */ @@ -52,4 +56,8 @@ MEM_INLINE bool MEM_size_safe_multiply(size_t a, size_t b, size_t *result) return ((high_bits & (a | b)) == 0 || (*result / b == a)); } +#ifdef __cplusplus +} +#endif + #endif /* __MALLOCN_INLINE_H__ */ diff --git a/intern/guardedalloc/intern/mallocn_intern.h b/intern/guardedalloc/intern/mallocn_intern.h index ef8845a66b3..8fc3e432157 100644 --- a/intern/guardedalloc/intern/mallocn_intern.h +++ b/intern/guardedalloc/intern/mallocn_intern.h @@ -100,11 +100,18 @@ size_t malloc_usable_size(void *ptr); #include "mallocn_inline.h" +#ifdef __cplusplus +extern "C" { +#endif + #define ALIGNED_MALLOC_MINIMUM_ALIGNMENT sizeof(void *) void *aligned_malloc(size_t size, size_t alignment); void aligned_free(void *ptr); +extern bool leak_detector_has_run; +extern char free_after_leak_detection_message[]; + /* Prototypes for counted allocator functions */ size_t MEM_lockfree_allocN_len(const void *vmemh) ATTR_WARN_UNUSED_RESULT; void MEM_lockfree_freeN(void *vmemh); @@ -191,4 +198,8 @@ size_t MEM_guarded_get_peak_memory(void) ATTR_WARN_UNUSED_RESULT; const char *MEM_guarded_name_ptr(void *vmemh); #endif +#ifdef __cplusplus +} +#endif + #endif /* __MALLOCN_INTERN_H__ */ diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c index 205cc688d72..282c852fedd 100644 --- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c +++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c @@ -101,6 +101,10 @@ size_t MEM_lockfree_allocN_len(const void *vmemh) void MEM_lockfree_freeN(void *vmemh) { + if (leak_detector_has_run) { + print_error(free_after_leak_detection_message); + } + MemHead *memh = MEMHEAD_FROM_PTR(vmemh); size_t len = MEM_lockfree_allocN_len(vmemh); diff --git a/source/blender/makesdna/intern/CMakeLists.txt b/source/blender/makesdna/intern/CMakeLists.txt index 0f2761e311e..737ea9a7e12 100644 --- a/source/blender/makesdna/intern/CMakeLists.txt +++ b/source/blender/makesdna/intern/CMakeLists.txt @@ -40,6 +40,7 @@ set(SRC ../../blenlib/intern/BLI_memarena.c ../../blenlib/intern/BLI_mempool.c ../../blenlib/intern/hash_mm2a.c # needed by 'BLI_ghash_utils.c', not used directly. + ../../../../intern/guardedalloc/intern/leak_detector.cc ../../../../intern/guardedalloc/intern/mallocn.c ../../../../intern/guardedalloc/intern/mallocn_guarded_impl.c ../../../../intern/guardedalloc/intern/mallocn_lockfree_impl.c diff --git a/source/blender/makesrna/intern/CMakeLists.txt b/source/blender/makesrna/intern/CMakeLists.txt index a6ed705ec67..0b43a5a6653 100644 --- a/source/blender/makesrna/intern/CMakeLists.txt +++ b/source/blender/makesrna/intern/CMakeLists.txt @@ -175,6 +175,7 @@ set(SRC ${DEFSRC} ${APISRC} ../../../../intern/clog/clog.c + ../../../../intern/guardedalloc/intern/leak_detector.cc ../../../../intern/guardedalloc/intern/mallocn.c ../../../../intern/guardedalloc/intern/mallocn_guarded_impl.c ../../../../intern/guardedalloc/intern/mallocn_lockfree_impl.c diff --git a/source/blender/makesrna/intern/makesrna.c b/source/blender/makesrna/intern/makesrna.c index b34c324bd91..045d098bf6a 100644 --- a/source/blender/makesrna/intern/makesrna.c +++ b/source/blender/makesrna/intern/makesrna.c @@ -5144,7 +5144,10 @@ static void mem_error_cb(const char *errorStr) int main(int argc, char **argv) { - int totblock, return_status = 0; + int return_status = 0; + + MEM_initialize_memleak_detection(); + MEM_set_error_callback(mem_error_cb); CLG_init(); @@ -5166,12 +5169,5 @@ int main(int argc, char **argv) CLG_exit(); - totblock = MEM_get_memory_blocks_in_use(); - if (totblock != 0) { - fprintf(stderr, "Error Totblock: %d\n", totblock); - MEM_set_error_callback(mem_error_cb); - MEM_printmemlist(); - } - return return_status; } diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c index aa3f11c2515..945d5fd42e4 100644 --- a/source/blender/windowmanager/intern/wm_init_exit.c +++ b/source/blender/windowmanager/intern/wm_init_exit.c @@ -656,13 +656,6 @@ void WM_exit_ex(bContext *C, const bool do_python) BKE_blender_atexit(); - if (MEM_get_memory_blocks_in_use() != 0) { - size_t mem_in_use = MEM_get_memory_in_use() + MEM_get_memory_in_use(); - printf("Error: Not freed memory blocks: %u, total unfreed memory %f MB\n", - MEM_get_memory_blocks_in_use(), - (double)mem_in_use / 1024 / 1024); - MEM_printmemlist(); - } wm_autosave_delete(); BKE_tempdir_session_purge(); diff --git a/source/creator/creator.c b/source/creator/creator.c index abc443425a8..9268ed15923 100644 --- a/source/creator/creator.c +++ b/source/creator/creator.c @@ -295,6 +295,7 @@ int main(int argc, break; } } + MEM_initialize_memleak_detection(); } #ifdef BUILD_DATE diff --git a/tests/gtests/blenloader/blendfile_loading_base_test.cc b/tests/gtests/blenloader/blendfile_loading_base_test.cc index f15ae615e8a..1f3a312de5d 100644 --- a/tests/gtests/blenloader/blendfile_loading_base_test.cc +++ b/tests/gtests/blenloader/blendfile_loading_base_test.cc @@ -100,14 +100,6 @@ void BlendfileLoadingBaseTest::TearDownTestCase() BKE_blender_atexit(); - if (MEM_get_memory_blocks_in_use() != 0) { - size_t mem_in_use = MEM_get_memory_in_use() + MEM_get_memory_in_use(); - printf("Error: Not freed memory blocks: %u, total unfreed memory %f MB\n", - MEM_get_memory_blocks_in_use(), - (double)mem_in_use / 1024 / 1024); - MEM_printmemlist(); - } - BKE_tempdir_session_purge(); testing::Test::TearDownTestCase(); diff --git a/tests/gtests/testing/CMakeLists.txt b/tests/gtests/testing/CMakeLists.txt index c8a7f487c5d..d557b27f272 100644 --- a/tests/gtests/testing/CMakeLists.txt +++ b/tests/gtests/testing/CMakeLists.txt @@ -28,6 +28,7 @@ set(INC ${GLOG_INCLUDE_DIRS} ${GFLAGS_INCLUDE_DIRS} ../../../extern/gtest/include + ../../../intern/guardedalloc ) set(INC_SYS @@ -40,6 +41,7 @@ set(SRC ) set(LIB + bf_intern_guardedalloc ) blender_add_lib(bf_testing_main "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") diff --git a/tests/gtests/testing/testing_main.cc b/tests/gtests/testing/testing_main.cc index 0acdcf3a8a5..9816e71526b 100644 --- a/tests/gtests/testing/testing_main.cc +++ b/tests/gtests/testing/testing_main.cc @@ -19,6 +19,8 @@ #include "testing/testing.h" +#include "MEM_guardedalloc.h" + DEFINE_string(test_assets_dir, "", "lib/tests directory from SVN containing the test assets."); DEFINE_string(test_release_dir, "", "bin/{blender version} directory of the current build."); @@ -46,6 +48,7 @@ const std::string &flags_test_release_dir() int main(int argc, char **argv) { + MEM_initialize_memleak_detection(); testing::InitGoogleTest(&argc, argv); BLENDER_GFLAGS_NAMESPACE::ParseCommandLineFlags(&argc, &argv, true); google::InitGoogleLogging(argv[0]); -- cgit v1.2.3 From ec17b45034fbc9278bb42ea574bdaf2b8a6857e3 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Fri, 24 Jul 2020 12:38:04 +0200 Subject: Depsgraph: use construct on first use idiom for graph registry This is necessary to avoid false positive memory leaks. --- source/blender/depsgraph/intern/depsgraph_registry.cc | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/source/blender/depsgraph/intern/depsgraph_registry.cc b/source/blender/depsgraph/intern/depsgraph_registry.cc index c9d03e47ded..623702ee3ae 100644 --- a/source/blender/depsgraph/intern/depsgraph_registry.cc +++ b/source/blender/depsgraph/intern/depsgraph_registry.cc @@ -30,29 +30,35 @@ namespace blender { namespace deg { -static RawMap
> g_graph_registry; +using GraphRegistry = Map
>; +static GraphRegistry &get_graph_registry() +{ + static GraphRegistry graph_registry; + return graph_registry; +} void register_graph(Depsgraph *depsgraph) { Main *bmain = depsgraph->bmain; - g_graph_registry.lookup_or_add_default(bmain).add_new(depsgraph); + get_graph_registry().lookup_or_add_default(bmain).add_new(depsgraph); } void unregister_graph(Depsgraph *depsgraph) { Main *bmain = depsgraph->bmain; - RawVectorSet &graphs = g_graph_registry.lookup(bmain); + GraphRegistry &graph_registry = get_graph_registry(); + VectorSet &graphs = graph_registry.lookup(bmain); graphs.remove(depsgraph); // If this was the last depsgraph associated with the main, remove the main entry as well. if (graphs.is_empty()) { - g_graph_registry.remove(bmain); + graph_registry.remove(bmain); } } Span get_all_registered_graphs(Main *bmain) { - RawVectorSet *graphs = g_graph_registry.lookup_ptr(bmain); + VectorSet *graphs = get_graph_registry().lookup_ptr(bmain); if (graphs != nullptr) { return *graphs; } -- cgit v1.2.3