From 8a9912eaf81bab73a12621a4c0987c37a865fe50 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Wed, 26 Aug 2020 22:02:02 +0200 Subject: Tests: fail automated tests on memory leaks and other internal errors This adds a new `--debug-exit-on-error` flag. When it is set, Blender will abort with a non-zero exit code when there are internal errors. Currently, "internal errors" includes memory leaks detected by guardedalloc and error/fatal log entries in clog. The new flag is passed to Blender in various places where automated tests are run. Furthermore, the `--debug-memory` flag is used in tests, because that makes the verbose output more useful, when dealing with memory leaks. Reviewers: brecht, sergey Differential Revision: https://developer.blender.org/D8665 --- intern/clog/CLG_log.h | 1 + intern/clog/clog.c | 23 +++++++++++++++++++++++ intern/guardedalloc/MEM_guardedalloc.h | 5 +++++ intern/guardedalloc/intern/leak_detector.cc | 19 +++++++++++++++++++ source/creator/creator_args.c | 20 ++++++++++++++++++++ tests/CMakeLists.txt | 2 +- tests/gtests/testing/testing_main.cc | 1 + tests/python/collada/CMakeLists.txt | 6 +++--- tests/python/cycles_render_tests.py | 2 ++ tests/python/eevee_render_tests.py | 2 ++ tests/python/modules/render_report.py | 11 ++++++----- tests/python/modules/test_utils.py | 2 ++ tests/python/opengl_draw_tests.py | 2 ++ tests/python/view_layer/CMakeLists.txt | 2 +- tests/python/workbench_render_tests.py | 2 ++ 15 files changed, 90 insertions(+), 10 deletions(-) diff --git a/intern/clog/CLG_log.h b/intern/clog/CLG_log.h index 35515a1d5c6..a2841c5c8b3 100644 --- a/intern/clog/CLG_log.h +++ b/intern/clog/CLG_log.h @@ -139,6 +139,7 @@ void CLG_exit(void); void CLG_output_set(void *file_handle); void CLG_output_use_basename_set(int value); void CLG_output_use_timestamp_set(int value); +void CLG_error_fn_set(void (*error_fn)(void *file_handle)); void CLG_fatal_fn_set(void (*fatal_fn)(void *file_handle)); void CLG_backtrace_fn_set(void (*fatal_fn)(void *file_handle)); diff --git a/intern/clog/clog.c b/intern/clog/clog.c index d384b9a89e6..84b850f5042 100644 --- a/intern/clog/clog.c +++ b/intern/clog/clog.c @@ -98,6 +98,7 @@ typedef struct CLogContext { } default_type; struct { + void (*error_fn)(void *file_handle); void (*fatal_fn)(void *file_handle); void (*backtrace_fn)(void *file_handle); } callbacks; @@ -352,6 +353,13 @@ static CLG_LogType *clg_ctx_type_register(CLogContext *ctx, const char *identifi return ty; } +static void clg_ctx_error_action(CLogContext *ctx) +{ + if (ctx->callbacks.error_fn != NULL) { + ctx->callbacks.error_fn(ctx->output_file); + } +} + static void clg_ctx_fatal_action(CLogContext *ctx) { if (ctx->callbacks.fatal_fn != NULL) { @@ -522,6 +530,10 @@ void CLG_logf(CLG_LogType *lg, clg_ctx_backtrace(lg->ctx); } + if (severity == CLG_SEVERITY_ERROR) { + clg_ctx_error_action(lg->ctx); + } + if (severity == CLG_SEVERITY_FATAL) { clg_ctx_fatal_action(lg->ctx); } @@ -555,6 +567,12 @@ static void CLG_ctx_output_use_timestamp_set(CLogContext *ctx, int value) } } +/** Action on error severity. */ +static void CLT_ctx_error_fn_set(CLogContext *ctx, void (*error_fn)(void *file_handle)) +{ + ctx->callbacks.error_fn = error_fn; +} + /** Action on fatal severity. */ static void CLG_ctx_fatal_fn_set(CLogContext *ctx, void (*fatal_fn)(void *file_handle)) { @@ -674,6 +692,11 @@ void CLG_output_use_timestamp_set(int value) CLG_ctx_output_use_timestamp_set(g_ctx, value); } +void CLG_error_fn_set(void (*error_fn)(void *file_handle)) +{ + CLT_ctx_error_fn_set(g_ctx, error_fn); +} + void CLG_fatal_fn_set(void (*fatal_fn)(void *file_handle)) { CLG_ctx_fatal_fn_set(g_ctx, fatal_fn); diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h index 9c62b2396f6..c05bda030ad 100644 --- a/intern/guardedalloc/MEM_guardedalloc.h +++ b/intern/guardedalloc/MEM_guardedalloc.h @@ -215,6 +215,11 @@ extern const char *(*MEM_name_ptr)(void *vmemh); * about memory leaks will be printed on exit. */ void MEM_init_memleak_detection(void); +/** When this has been called and memory leaks have been detected, the process will have an exit + * code that indicates failure. This can be used for when checking for memory leaks with automated + * tests. */ +void MEM_enable_fail_on_memleak(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 index d7b6f749742..bb816e7f2d3 100644 --- a/intern/guardedalloc/intern/leak_detector.cc +++ b/intern/guardedalloc/intern/leak_detector.cc @@ -18,6 +18,8 @@ * \ingroup MEM */ +#include + #include "MEM_guardedalloc.h" #include "mallocn_intern.h" @@ -28,6 +30,9 @@ char free_after_leak_detection_message[] = "error, use the 'construct on first use' idiom."; namespace { + +static bool fail_on_memleak = false; + class MemLeakPrinter { public: ~MemLeakPrinter() @@ -42,6 +47,15 @@ class MemLeakPrinter { leaked_blocks, (double)mem_in_use / 1024 / 1024); MEM_printmemlist(); + + if (fail_on_memleak) { + /* There are many other ways to change the exit code to failure here: + * - Make the destructor noexcept(false) and throw an exception. + * - Call exit(EXIT_FAILURE). + * - Call terminate(). + */ + abort(); + } } }; } // namespace @@ -59,3 +73,8 @@ void MEM_init_memleak_detection(void) */ static MemLeakPrinter printer; } + +void MEM_enable_fail_on_memleak(void) +{ + fail_on_memleak = true; +} diff --git a/source/creator/creator_args.c b/source/creator/creator_args.c index e199030ef71..0d1c932d2d2 100644 --- a/source/creator/creator_args.c +++ b/source/creator/creator_args.c @@ -751,6 +751,25 @@ static int arg_handle_abort_handler_disable(int UNUSED(argc), return 0; } +static void clog_abort_on_error_callback(void *fp) +{ + BLI_system_backtrace(fp); + fflush(fp); + abort(); +} + +static const char arg_handle_debug_exit_on_error_doc[] = + "\n\t" + "Immediately exit when internal errors are detected."; +static int arg_handle_debug_exit_on_error(int UNUSED(argc), + const char **UNUSED(argv), + void *UNUSED(data)) +{ + MEM_enable_fail_on_memleak(); + CLG_error_fn_set(clog_abort_on_error_callback); + return 0; +} + static const char arg_handle_background_mode_set_doc[] = "\n\t" "Run in background (often used for UI-less rendering)."; @@ -2214,6 +2233,7 @@ void main_args_setup(bContext *C, bArgs *ba) "--debug-gpu-force-workarounds", CB_EX(arg_handle_debug_mode_generic_set, gpumem), (void *)G_DEBUG_GPU_FORCE_WORKAROUNDS); + BLI_argsAdd(ba, 1, NULL, "--debug-exit-on-error", CB(arg_handle_debug_exit_on_error), NULL); BLI_argsAdd(ba, 1, NULL, "--verbose", CB(arg_handle_verbosity_set), NULL); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0ee3b500fdf..8f72db07d92 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -44,7 +44,7 @@ unset(_default_test_python_exe) # Standard Blender arguments for running tests. # Specify exit code so that if a Python script error happens, the test fails. -set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --python-exit-code 1) +set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --debug-memory --debug-exit-on-error --python-exit-code 1) # Python CTests if(WITH_BLENDER AND WITH_PYTHON) diff --git a/tests/gtests/testing/testing_main.cc b/tests/gtests/testing/testing_main.cc index e9313b31909..6238101b319 100644 --- a/tests/gtests/testing/testing_main.cc +++ b/tests/gtests/testing/testing_main.cc @@ -50,6 +50,7 @@ int main(int argc, char **argv) { MEM_use_guarded_allocator(); MEM_init_memleak_detection(); + MEM_enable_fail_on_memleak(); testing::InitGoogleTest(&argc, argv); BLENDER_GFLAGS_NAMESPACE::ParseCommandLineFlags(&argc, &argv, true); google::InitGoogleLogging(argv[0]); diff --git a/tests/python/collada/CMakeLists.txt b/tests/python/collada/CMakeLists.txt index bf22de6b762..1b24875578e 100644 --- a/tests/python/collada/CMakeLists.txt +++ b/tests/python/collada/CMakeLists.txt @@ -36,12 +36,12 @@ execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory ${TEST_OUT_DIR}) # all calls to blender use this if(APPLE) if(${CMAKE_GENERATOR} MATCHES "Xcode") - set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup) + set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --debug-memory --debug-exit-on-error) else() - set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) + set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --debug-memory --debug-exit-on-error --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) endif() else() - set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) + set(TEST_BLENDER_EXE_PARAMS --background -noaudio --factory-startup --debug-memory --debug-exit-on-error --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) endif() # for testing with valgrind prefix: valgrind --track-origins=yes --error-limit=no diff --git a/tests/python/cycles_render_tests.py b/tests/python/cycles_render_tests.py index bdf4283eb3e..cc949248ce6 100644 --- a/tests/python/cycles_render_tests.py +++ b/tests/python/cycles_render_tests.py @@ -20,6 +20,8 @@ def get_arguments(filepath, output_filepath): "-noaudio", "--factory-startup", "--enable-autoexec", + "--debug-memory", + "--debug-exit-on-error", filepath, "-E", "CYCLES", "-o", output_filepath, diff --git a/tests/python/eevee_render_tests.py b/tests/python/eevee_render_tests.py index a7130136d0a..a90d7730ace 100644 --- a/tests/python/eevee_render_tests.py +++ b/tests/python/eevee_render_tests.py @@ -103,6 +103,8 @@ def get_arguments(filepath, output_filepath): "-noaudio", "--factory-startup", "--enable-autoexec", + "--debug-memory", + "--debug-exit-on-error", filepath, "-E", "BLENDER_EEVEE", "-P", diff --git a/tests/python/modules/render_report.py b/tests/python/modules/render_report.py index 506c1a1518a..b6cafc2ee24 100755 --- a/tests/python/modules/render_report.py +++ b/tests/python/modules/render_report.py @@ -448,16 +448,17 @@ class Report: crash = False output = None try: - output = subprocess.check_output(command) - except subprocess.CalledProcessError as e: - crash = True + completed_process = subprocess.run(command, stdout=subprocess.PIPE) + if completed_process.returncode != 0: + crash = True + output = completed_process.stdout except BaseException as e: crash = True if verbose: print(" ".join(command)) - if output: - print(output.decode("utf-8")) + if (verbose or crash) and output: + print(output.decode("utf-8")) # Detect missing filepaths and consider those errors for filepath, output_filepath in zip(remaining_filepaths[:], output_filepaths): diff --git a/tests/python/modules/test_utils.py b/tests/python/modules/test_utils.py index e31db05ba61..9c23d511813 100755 --- a/tests/python/modules/test_utils.py +++ b/tests/python/modules/test_utils.py @@ -79,6 +79,8 @@ class AbstractBlenderRunnerTest(unittest.TestCase): '-noaudio', '--factory-startup', '--enable-autoexec', + '--debug-memory', + '--debug-exit-on-error', ] if blendfile: diff --git a/tests/python/opengl_draw_tests.py b/tests/python/opengl_draw_tests.py index ab4df63afd9..a2d9d510382 100644 --- a/tests/python/opengl_draw_tests.py +++ b/tests/python/opengl_draw_tests.py @@ -39,6 +39,8 @@ def get_arguments(filepath, output_filepath): "-noaudio", "--factory-startup", "--enable-autoexec", + "--debug-memory", + "--debug-exit-on-error", filepath, "-P", os.path.realpath(__file__), diff --git a/tests/python/view_layer/CMakeLists.txt b/tests/python/view_layer/CMakeLists.txt index 3f38ee4675f..3cac2b6d85f 100644 --- a/tests/python/view_layer/CMakeLists.txt +++ b/tests/python/view_layer/CMakeLists.txt @@ -30,7 +30,7 @@ execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory ${TEST_OUT_DIR}) # endif() # for testing with valgrind prefix: valgrind --track-origins=yes --error-limit=no -set(TEST_BLENDER_EXE $ --background -noaudio --factory-startup --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) +set(TEST_BLENDER_EXE $ --background -noaudio --factory-startup --debug-memory --debug-exit-on-error --env-system-scripts ${CMAKE_SOURCE_DIR}/release/scripts) # ------------------------------------------------------------------------------ diff --git a/tests/python/workbench_render_tests.py b/tests/python/workbench_render_tests.py index 155b54098a8..7c9842d5733 100644 --- a/tests/python/workbench_render_tests.py +++ b/tests/python/workbench_render_tests.py @@ -39,6 +39,8 @@ def get_arguments(filepath, output_filepath): "-noaudio", "--factory-startup", "--enable-autoexec", + "--debug-memory", + "--debug-exit-on-error", filepath, "-E", "BLENDER_WORKBENCH", "-P", -- cgit v1.2.3