From 6e5c5255b080098c1efa2ea386c1ea8a3170c4e1 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Tue, 4 Aug 2026 15:50:28 -0600 Subject: [PATCH] add multithreaded shutdown stress test --- .github/workflows/dynamic_arch.yml | 4 +- cpp_thread_test/CMakeLists.txt | 13 ++ cpp_thread_test/Makefile | 11 +- .../dgemm_thread_safety_shutdown.cpp | 165 ++++++++++++++++++ 4 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 cpp_thread_test/dgemm_thread_safety_shutdown.cpp diff --git a/.github/workflows/dynamic_arch.yml b/.github/workflows/dynamic_arch.yml index 67919abe4..f7d63d8c8 100644 --- a/.github/workflows/dynamic_arch.yml +++ b/.github/workflows/dynamic_arch.yml @@ -593,7 +593,7 @@ jobs: - name: Build OpenBLAS run: | cd build - cmake --build . --target dgemm_thread_safety dgemm_thread_safety_mixed dgemv_thread_safety + cmake --build . --target dgemm_thread_safety dgemm_thread_safety_mixed dgemm_thread_safety_shutdown dgemv_thread_safety - name: Show ccache status continue-on-error: true @@ -611,7 +611,7 @@ jobs: run: | cd build export PATH="$PWD/lib:$PATH" - OPENBLAS_NUM_THREADS=8 OMP_NUM_THREADS=16 ctest -R 'dgemm_thread_safety|dgemm_thread_safety_mixed|dgemv_thread_safety' --output-on-failure + OPENBLAS_NUM_THREADS=8 OMP_NUM_THREADS=16 ctest -R 'dgemm_thread_safety|dgemm_thread_safety_mixed|dgemm_thread_safety_shutdown|dgemv_thread_safety' --output-on-failure cross_build: diff --git a/cpp_thread_test/CMakeLists.txt b/cpp_thread_test/CMakeLists.txt index 5271d4594..0f140f25f 100644 --- a/cpp_thread_test/CMakeLists.txt +++ b/cpp_thread_test/CMakeLists.txt @@ -19,6 +19,7 @@ endif() set(CPP_THREAD_SAFETY_DGEMM_ARGS "" CACHE STRING "Arguments passed to the DGEMM thread safety test") set(CPP_THREAD_SAFETY_DGEMM_MIXED_ARGS "" CACHE STRING "Arguments passed to the mixed DGEMM thread safety test") set(CPP_THREAD_SAFETY_DGEMV_ARGS "" CACHE STRING "Arguments passed to the DGEMV thread safety test") +set(CPP_THREAD_SAFETY_SHUTDOWN_ARGS "" CACHE STRING "Arguments passed to the DGEMM shutdown safety test") if (CPP_THREAD_SAFETY_TEST) message(STATUS "building thread safety test") @@ -29,6 +30,18 @@ if (CPP_THREAD_SAFETY_TEST) add_executable(dgemm_thread_safety_mixed dgemm_thread_safety_mixed.cpp) target_link_libraries(dgemm_thread_safety_mixed ${CPP_THREAD_SAFETY_LIBS}) add_test(NAME dgemm_thread_safety_mixed COMMAND ${CMAKE_CURRENT_BINARY_DIR}/dgemm_thread_safety_mixed ${CPP_THREAD_SAFETY_DGEMM_MIXED_ARGS}) + + # The shutdown race is Windows-specific: on POSIX, exit() runs the library + # destructor while worker threads are still computing into OpenBLAS-owned + # buffers, which no amount of locking inside blas_shutdown can make safe. + if (WIN32) + add_executable(dgemm_thread_safety_shutdown dgemm_thread_safety_shutdown.cpp) + target_link_libraries(dgemm_thread_safety_shutdown ${CPP_THREAD_SAFETY_LIBS}) + add_test(NAME dgemm_thread_safety_shutdown COMMAND ${CMAKE_CURRENT_BINARY_DIR}/dgemm_thread_safety_shutdown ${CPP_THREAD_SAFETY_SHUTDOWN_ARGS}) + # Bounded by the test itself: 40 children, each killed after 10s at worst. + # A passing run takes a few seconds; only a failing one approaches this. + set_tests_properties(dgemm_thread_safety_shutdown PROPERTIES TIMEOUT 600) + endif() endif() diff --git a/cpp_thread_test/Makefile b/cpp_thread_test/Makefile index fe7a28625..1aa04094b 100644 --- a/cpp_thread_test/Makefile +++ b/cpp_thread_test/Makefile @@ -3,6 +3,11 @@ include $(TOPDIR)/Makefile.system all :: dgemv_tester dgemm_tester dgemm_mixed_tester +# The shutdown race is Windows-specific; see dgemm_thread_safety_shutdown.cpp. +ifeq ($(OSNAME), WINNT) +all :: dgemm_shutdown_tester +endif + dgemv_tester : $(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemv_thread_safety.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemv_tester ./dgemv_tester @@ -15,5 +20,9 @@ dgemm_mixed_tester : dgemm_tester $(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemm_thread_safety_mixed.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemm_mixed_tester ./dgemm_mixed_tester +dgemm_shutdown_tester : dgemm_mixed_tester + $(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemm_thread_safety_shutdown.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemm_shutdown_tester + ./dgemm_shutdown_tester + clean :: - rm -f dgemv_tester dgemm_tester dgemm_mixed_tester + rm -f dgemv_tester dgemm_tester dgemm_mixed_tester dgemm_shutdown_tester diff --git a/cpp_thread_test/dgemm_thread_safety_shutdown.cpp b/cpp_thread_test/dgemm_thread_safety_shutdown.cpp new file mode 100644 index 000000000..4afc7828f --- /dev/null +++ b/cpp_thread_test/dgemm_thread_safety_shutdown.cpp @@ -0,0 +1,165 @@ +/* Stress test for library shutdown racing with in-flight BLAS calls + * (https://github.com/OpenMathLib/OpenBLAS/issues/5954). + * + * Windows only. On POSIX, exit() runs the library destructor while worker + * threads are still computing into OpenBLAS-owned buffers, which no amount of + * locking inside blas_shutdown can make safe, so there is nothing to assert + * there; CMakeLists.txt only registers this test on WIN32. + * + * The parent re-executes itself as short-lived children and checks that each + * one terminates cleanly, turning shutdown-path crashes and deadlocks into + * ordinary test failures. Each child (--child-storm N) starts N callers that + * allocate their matrices and park on a gate, releases them so they all enter + * their first dgemm at once, and exits a millisecond later while that + * allocation storm is still in flight. + * + * N must exceed NUM_BUFFERS = MAX(50, NUM_THREADS * 2 * NUM_PARALLEL) for the + * build under test; below that every slot is already mapped and the race is + * unreachable. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef OPENBLAS_USE_GENERATED_CBLAS_H +#include "generated/cblas.h" +#else +#include "../cblas.h" +#endif + +#include + +namespace { + +const blasint stormM = 200, stormK = 120, stormN = 90; /* the gh-5954 shape */ +const blasint poolDim = 320; /* above the multithreading threshold, so the pool spins up */ +const uint32_t defaultStormCallers = 128; +const uint32_t stormDelayMs = 3; /* gate to sweep; at 0 the sweep beats the allocations */ +const int stormBlasThreads = 4; +const int stormTimeoutSec = 10; +const int numStormChildren = 40; + +std::atomic parked(0); /* callers built and waiting on the gate */ +std::atomic gate(false); + +void fillOperands(std::vector& A, std::vector& B) { + for (size_t i = 0; i < A.size(); i++) A[i] = (i % 1000) / 1000.0; + for (size_t i = 0; i < B.size(); i++) B[i] = (i % 997) / 997.0; +} + +void dgemmOnce(blasint m, blasint k, blasint n) { + std::vector A(m * k), B(k * n), C(m * n); + fillOperands(A, B); + cblas_dgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, + 1.0, A.data(), m, B.data(), k, 0.1, C.data(), m); +} + +/* Allocate before parking, so that when the gate opens nothing stands between + the thread and its first dgemm. */ +void gatedWorker(blasint m, blasint k, blasint n) { + std::vector A(m * k), B(k * n), C(m * n); + fillOperands(A, B); + parked.fetch_add(1, std::memory_order_release); + while (!gate.load(std::memory_order_acquire)) std::this_thread::yield(); + for (;;) + cblas_dgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k, + 1.0, A.data(), m, B.data(), k, 0.1, C.data(), m); +} + +int ChildStorm(uint32_t nCallers) { + SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX); + openblas_set_num_threads(stormBlasThreads); + + /* Build the OpenBLAS worker pool first, so the storm is buffer allocation + and not pool startup. */ + dgemmOnce(poolDim, poolDim, poolDim); + + for (uint32_t i = 0; i < nCallers; i++) + std::thread(gatedWorker, stormM, stormK, stormN).detach(); + for (int ms = 0; parked.load(std::memory_order_acquire) < nCallers && ms < 10000; ms++) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + + gate.store(true, std::memory_order_release); + std::this_thread::sleep_for(std::chrono::milliseconds(stormDelayMs)); + std::exit(0); +} + +/* Returns 0 if the child exited cleanly, nonzero otherwise; fills outcome. */ +int RunChild(const std::string& args, int timeoutSec, std::string& outcome) { + char exe[MAX_PATH]; + if (GetModuleFileNameA(NULL, exe, MAX_PATH) == 0) { + outcome = "GetModuleFileName failed"; + return 1; + } + std::string cmd = "\"" + std::string(exe) + "\" " + args; + + STARTUPINFOA si; + PROCESS_INFORMATION pi; + ZeroMemory(&si, sizeof(si)); + si.cb = sizeof(si); + ZeroMemory(&pi, sizeof(pi)); + if (!CreateProcessA(NULL, &cmd[0], NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) { + outcome = "CreateProcess failed"; + return 1; + } + + int ret = 1; + char buf[64]; + if (WaitForSingleObject(pi.hProcess, timeoutSec * 1000) != WAIT_OBJECT_0) { + TerminateProcess(pi.hProcess, 1); + WaitForSingleObject(pi.hProcess, 5000); + snprintf(buf, sizeof(buf), "HANG (killed after %ds)", timeoutSec); + } else { + DWORD code = 1; + GetExitCodeProcess(pi.hProcess, &code); + if (code == 0) { + snprintf(buf, sizeof(buf), "clean exit"); + ret = 0; + } else { + snprintf(buf, sizeof(buf), "CRASH (exit code 0x%08lX)", (unsigned long)code); + } + } + outcome = buf; + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); + return ret; +} + +} // namespace + +int main(int argc, char* argv[]) { + if (argc >= 3 && std::strcmp(argv[1], "--child-storm") == 0) + return ChildStorm(uint32_t(std::atoi(argv[2]))); + SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX); + + uint32_t callers = defaultStormCallers; + if (argc >= 2) { + int n = std::atoi(argv[1]); + if (n > 0) callers = uint32_t(n); + } + + int failures = 0; + std::cout << "Testing process exit during an allocation storm (" << callers << " callers)" + << std::endl; + for (int i = 0; i < numStormChildren; i++) { + std::string outcome; + failures += RunChild("--child-storm " + std::to_string(callers), stormTimeoutSec, outcome); + std::cout << " storm child " << i << ": " << outcome << std::endl; + } + + if (failures) { + std::cout << "CBLAS DGEMM shutdown safety test FAILED! (" << failures + << " child processes)" << std::endl; + return 1; + } + std::cout << "CBLAS DGEMM shutdown safety test PASSED!" << std::endl; + return 0; +}