vld leak check at exit#341
Open
anporumb wants to merge 9 commits into
Open
Conversation
RunTests sampled VLDGetLeaksCount() at the end of the suite and encoded the delta as the process exit code. That sampling ran before C++ static destructors, so function-local process-lifetime statics (for example azure-core's Url::Encode character lookup set, built lazily during a storage test) were still alive and got misreported as leaks, failing the suite with a spurious negative exit code. This showed up as exit code -8 on the ARM64 Debug gate for async_wrapper_int, while the same suite passed in RelWithDebInfo where VLD's API is compiled out under NDEBUG. Defer the leak verdict to an atexit handler registered once at the first RunTests entry. It runs at process exit, after all static destructors, so process-lifetime statics are already freed and excluded, while allocations that genuinely survive teardown are still caught and force a non-zero exit. The end-of-RunTests delta and the now superseded retry loop are removed; the useLeakCheckRetries parameter is retained for API compatibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests/ctest_leak_check_int covering ctest_check_leaks_at_exit: - clean (C): malloc + free exits 0. - real_leak (C): unfreed malloc forces a non-zero exit (WILL_FAIL). - real_leak_cpp (C++): unfreed operator new forces a non-zero exit (WILL_FAIL). - process_lifetime_static (C++): a lazily-initialized, function-local process-lifetime std::unordered_set (the azure-core Url::Encode pattern) constructed during a test must exit 0. This is the regression case for the previous end-of-RunTests sampling that misreported it as a leak (exit -8 on the ARM64 Debug gate). C++ suites live in .cpp files with a separate C runner main, because CTEST_RUN_TEST_SUITE expands to C-linkage code that must be at global scope. Tests are registered only when use_vld is enabled; the executables build in both flavors for compile coverage. Also harden ctest_check_leaks_at_exit: it runs at process exit, after main has returned and possibly after logger_deinit, and logger_log aborts when the logger is not initialized. Write the leak diagnostic to stderr instead of LogError to avoid aborting (observed as exit code 0xC0000409). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
RunTests is single-threaded (unguarded g_CurrentTestFunction/g_ExceptionJump globals), so InterlockedCompareExchange was unnecessary and misleadingly implied thread-safety. A static bool flag suffices to register the at-exit leak check once across the per-suite calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
… scope Extend tests/ctest_leak_check_int with process-lifetime allocation variants: - namespace_global: a C++ namespace-scope global (constructed before main, so captured in the leak baseline) is not reported. Exit 0. - dll_global_loadtime / loadlibrary / never_freed: a process-lifetime global in a VLD-instrumented helper DLL, reached via load-time linking, LoadLibrary then FreeLibrary, and LoadLibrary then never freed. Each is destroyed during teardown before the at-exit check, so none is reported. Exit 0. - dual_link: the same helper present both as a statically linked copy and as a separately loaded static-CRT DLL produces no spurious report. Exit 0. - dll_real_leak: a genuine leak in a VLD-instrumented (/MD) DLL is detected and fails the run. WILL_FAIL. - dll_mt_real_leak: a genuine leak in a static-CRT (/MT) DLL is NOT detected, documenting that such a DLL cannot be VLD-instrumented. Exit 0. Findings encoded by the tests: VLD only tracks allocations made by a module that itself links VLD, so the /MD helper DLL is instrumented via add_use_vld_if_no_cs (executables are instrumented by add_vld_if_defined, libraries are not). A /MT DLL cannot be instrumented because use_vld pulls the /MD CRT and conflicts (LNK4098), so leaks there are invisible. The static helper needs no instrumentation because it is linked into an executable that already carries VLD. C++ suites use a separate C runner main (CTEST_RUN_TEST_SUITE needs C linkage at global scope). All tests are registered only under use_vld and CONFIGURATIONS Debug, matching the existing leak-check tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
if(${run_unittests} OR ${run_int_tests}) expanded to a malformed if() when
run_int_tests was undefined (it is not declared as an option), producing "CMake
Error: Unknown arguments specified" at configure time. The CodeQL and
repo-validation gate jobs do not pass run_int_tests, so they failed at CMake
Generate; the test jobs pass it and were unaffected. Use bare variable names so
undefined variables evaluate to false, and harden the leak-check if(use_vld)
the same way.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The earlier generate fix switched to bare variable names. Instead, declare run_int_tests as an option defaulting OFF (mirroring run_unittests) so it always has a value, and restore the dollar-brace form in the if() guards. ctest's save/restore block captured run_int_tests before c-build-tools declared its option and restored the empty value, which is what left run_int_tests undefined at configure time; declaring the option in ctest's top CMakeLists before that block fixes it. use_vld is already a c-build-tools option and is not shadowed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The Linux leg builds with run_int_tests=ON and tried to compile ctest_leak_check_int, whose helpers use __declspec and LoadLibrary and whose whole purpose is the VLD leak check (Windows/MSVC only). gcc failed on __declspec(dllexport). Guard the subdirectory with if(WIN32) so the VLD tests are only built on Windows; the Linux build no longer sees them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jebrando
reviewed
Jun 12, 2026
| FOLDER "test_tools") | ||
|
|
||
| if (${run_unittests}) | ||
| if (${run_unittests} OR ${run_int_tests}) |
Contributor
There was a problem hiding this comment.
This will make it where if you run the int test it will also enable ut's even if they're not enabled. You need to update the tests CMakeLists.txt to make sure that either int or UT are enabled.
mattdurak
reviewed
Jun 12, 2026
| } | ||
| #endif | ||
|
|
||
| size_t RunTests(const TEST_FUNCTION_DATA* testListHead, const char* testSuiteName, bool useLeakCheckRetries, const char* testNameFilter) |
Contributor
mattdurak
reviewed
Jun 12, 2026
|
|
||
| add_executable(ctest_leak_check_dll_global_loadtime_int ctest_leak_check_dll_global_loadtime_int.cpp ctest_leak_check_dll_global_loadtime_int_main.c) | ||
| target_link_libraries(ctest_leak_check_dll_global_loadtime_int ctest ctest_leak_check_helper_dll) | ||
| set_target_properties(ctest_leak_check_dll_global_loadtime_int PROPERTIES FOLDER "tests/ctest") |
Contributor
There was a problem hiding this comment.
consider: we could make a cmake function for the leak checks that return 0 and another for the leak checks that fail and then reduce a ton of repeated lines here. /consider
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.