Skip to content

Commit 6e5c525

Browse files
committed
add multithreaded shutdown stress test
1 parent b6ee68d commit 6e5c525

4 files changed

Lines changed: 190 additions & 3 deletions

File tree

.github/workflows/dynamic_arch.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ jobs:
593593
- name: Build OpenBLAS
594594
run: |
595595
cd build
596-
cmake --build . --target dgemm_thread_safety dgemm_thread_safety_mixed dgemv_thread_safety
596+
cmake --build . --target dgemm_thread_safety dgemm_thread_safety_mixed dgemm_thread_safety_shutdown dgemv_thread_safety
597597
598598
- name: Show ccache status
599599
continue-on-error: true
@@ -611,7 +611,7 @@ jobs:
611611
run: |
612612
cd build
613613
export PATH="$PWD/lib:$PATH"
614-
OPENBLAS_NUM_THREADS=8 OMP_NUM_THREADS=16 ctest -R 'dgemm_thread_safety|dgemm_thread_safety_mixed|dgemv_thread_safety' --output-on-failure
614+
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
615615
616616
617617
cross_build:

cpp_thread_test/CMakeLists.txt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ endif()
1919
set(CPP_THREAD_SAFETY_DGEMM_ARGS "" CACHE STRING "Arguments passed to the DGEMM thread safety test")
2020
set(CPP_THREAD_SAFETY_DGEMM_MIXED_ARGS "" CACHE STRING "Arguments passed to the mixed DGEMM thread safety test")
2121
set(CPP_THREAD_SAFETY_DGEMV_ARGS "" CACHE STRING "Arguments passed to the DGEMV thread safety test")
22+
set(CPP_THREAD_SAFETY_SHUTDOWN_ARGS "" CACHE STRING "Arguments passed to the DGEMM shutdown safety test")
2223

2324
if (CPP_THREAD_SAFETY_TEST)
2425
message(STATUS "building thread safety test")
@@ -29,6 +30,18 @@ if (CPP_THREAD_SAFETY_TEST)
2930
add_executable(dgemm_thread_safety_mixed dgemm_thread_safety_mixed.cpp)
3031
target_link_libraries(dgemm_thread_safety_mixed ${CPP_THREAD_SAFETY_LIBS})
3132
add_test(NAME dgemm_thread_safety_mixed COMMAND ${CMAKE_CURRENT_BINARY_DIR}/dgemm_thread_safety_mixed ${CPP_THREAD_SAFETY_DGEMM_MIXED_ARGS})
33+
34+
# The shutdown race is Windows-specific: on POSIX, exit() runs the library
35+
# destructor while worker threads are still computing into OpenBLAS-owned
36+
# buffers, which no amount of locking inside blas_shutdown can make safe.
37+
if (WIN32)
38+
add_executable(dgemm_thread_safety_shutdown dgemm_thread_safety_shutdown.cpp)
39+
target_link_libraries(dgemm_thread_safety_shutdown ${CPP_THREAD_SAFETY_LIBS})
40+
add_test(NAME dgemm_thread_safety_shutdown COMMAND ${CMAKE_CURRENT_BINARY_DIR}/dgemm_thread_safety_shutdown ${CPP_THREAD_SAFETY_SHUTDOWN_ARGS})
41+
# Bounded by the test itself: 40 children, each killed after 10s at worst.
42+
# A passing run takes a few seconds; only a failing one approaches this.
43+
set_tests_properties(dgemm_thread_safety_shutdown PROPERTIES TIMEOUT 600)
44+
endif()
3245
endif()
3346

3447

cpp_thread_test/Makefile

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ include $(TOPDIR)/Makefile.system
33

44
all :: dgemv_tester dgemm_tester dgemm_mixed_tester
55

6+
# The shutdown race is Windows-specific; see dgemm_thread_safety_shutdown.cpp.
7+
ifeq ($(OSNAME), WINNT)
8+
all :: dgemm_shutdown_tester
9+
endif
10+
611
dgemv_tester :
712
$(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemv_thread_safety.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemv_tester
813
./dgemv_tester
@@ -15,5 +20,9 @@ dgemm_mixed_tester : dgemm_tester
1520
$(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemm_thread_safety_mixed.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemm_mixed_tester
1621
./dgemm_mixed_tester
1722

23+
dgemm_shutdown_tester : dgemm_mixed_tester
24+
$(CXX) $(COMMON_OPT) -Wall -Wextra -Wshadow -std=c++11 dgemm_thread_safety_shutdown.cpp ../$(LIBNAME) $(EXTRALIB) $(FEXTRALIB) -o dgemm_shutdown_tester
25+
./dgemm_shutdown_tester
26+
1827
clean ::
19-
rm -f dgemv_tester dgemm_tester dgemm_mixed_tester
28+
rm -f dgemv_tester dgemm_tester dgemm_mixed_tester dgemm_shutdown_tester
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/* Stress test for library shutdown racing with in-flight BLAS calls
2+
* (https://github.com/OpenMathLib/OpenBLAS/issues/5954).
3+
*
4+
* Windows only. On POSIX, exit() runs the library destructor while worker
5+
* threads are still computing into OpenBLAS-owned buffers, which no amount of
6+
* locking inside blas_shutdown can make safe, so there is nothing to assert
7+
* there; CMakeLists.txt only registers this test on WIN32.
8+
*
9+
* The parent re-executes itself as short-lived children and checks that each
10+
* one terminates cleanly, turning shutdown-path crashes and deadlocks into
11+
* ordinary test failures. Each child (--child-storm N) starts N callers that
12+
* allocate their matrices and park on a gate, releases them so they all enter
13+
* their first dgemm at once, and exits a millisecond later while that
14+
* allocation storm is still in flight.
15+
*
16+
* N must exceed NUM_BUFFERS = MAX(50, NUM_THREADS * 2 * NUM_PARALLEL) for the
17+
* build under test; below that every slot is already mapped and the race is
18+
* unreachable.
19+
*/
20+
#include <atomic>
21+
#include <chrono>
22+
#include <cstdint>
23+
#include <cstdio>
24+
#include <cstdlib>
25+
#include <cstring>
26+
#include <iostream>
27+
#include <string>
28+
#include <thread>
29+
#include <vector>
30+
31+
#ifdef OPENBLAS_USE_GENERATED_CBLAS_H
32+
#include "generated/cblas.h"
33+
#else
34+
#include "../cblas.h"
35+
#endif
36+
37+
#include <windows.h>
38+
39+
namespace {
40+
41+
const blasint stormM = 200, stormK = 120, stormN = 90; /* the gh-5954 shape */
42+
const blasint poolDim = 320; /* above the multithreading threshold, so the pool spins up */
43+
const uint32_t defaultStormCallers = 128;
44+
const uint32_t stormDelayMs = 3; /* gate to sweep; at 0 the sweep beats the allocations */
45+
const int stormBlasThreads = 4;
46+
const int stormTimeoutSec = 10;
47+
const int numStormChildren = 40;
48+
49+
std::atomic<uint32_t> parked(0); /* callers built and waiting on the gate */
50+
std::atomic<bool> gate(false);
51+
52+
void fillOperands(std::vector<double>& A, std::vector<double>& B) {
53+
for (size_t i = 0; i < A.size(); i++) A[i] = (i % 1000) / 1000.0;
54+
for (size_t i = 0; i < B.size(); i++) B[i] = (i % 997) / 997.0;
55+
}
56+
57+
void dgemmOnce(blasint m, blasint k, blasint n) {
58+
std::vector<double> A(m * k), B(k * n), C(m * n);
59+
fillOperands(A, B);
60+
cblas_dgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k,
61+
1.0, A.data(), m, B.data(), k, 0.1, C.data(), m);
62+
}
63+
64+
/* Allocate before parking, so that when the gate opens nothing stands between
65+
the thread and its first dgemm. */
66+
void gatedWorker(blasint m, blasint k, blasint n) {
67+
std::vector<double> A(m * k), B(k * n), C(m * n);
68+
fillOperands(A, B);
69+
parked.fetch_add(1, std::memory_order_release);
70+
while (!gate.load(std::memory_order_acquire)) std::this_thread::yield();
71+
for (;;)
72+
cblas_dgemm(CblasColMajor, CblasNoTrans, CblasNoTrans, m, n, k,
73+
1.0, A.data(), m, B.data(), k, 0.1, C.data(), m);
74+
}
75+
76+
int ChildStorm(uint32_t nCallers) {
77+
SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
78+
openblas_set_num_threads(stormBlasThreads);
79+
80+
/* Build the OpenBLAS worker pool first, so the storm is buffer allocation
81+
and not pool startup. */
82+
dgemmOnce(poolDim, poolDim, poolDim);
83+
84+
for (uint32_t i = 0; i < nCallers; i++)
85+
std::thread(gatedWorker, stormM, stormK, stormN).detach();
86+
for (int ms = 0; parked.load(std::memory_order_acquire) < nCallers && ms < 10000; ms++)
87+
std::this_thread::sleep_for(std::chrono::milliseconds(1));
88+
89+
gate.store(true, std::memory_order_release);
90+
std::this_thread::sleep_for(std::chrono::milliseconds(stormDelayMs));
91+
std::exit(0);
92+
}
93+
94+
/* Returns 0 if the child exited cleanly, nonzero otherwise; fills outcome. */
95+
int RunChild(const std::string& args, int timeoutSec, std::string& outcome) {
96+
char exe[MAX_PATH];
97+
if (GetModuleFileNameA(NULL, exe, MAX_PATH) == 0) {
98+
outcome = "GetModuleFileName failed";
99+
return 1;
100+
}
101+
std::string cmd = "\"" + std::string(exe) + "\" " + args;
102+
103+
STARTUPINFOA si;
104+
PROCESS_INFORMATION pi;
105+
ZeroMemory(&si, sizeof(si));
106+
si.cb = sizeof(si);
107+
ZeroMemory(&pi, sizeof(pi));
108+
if (!CreateProcessA(NULL, &cmd[0], NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
109+
outcome = "CreateProcess failed";
110+
return 1;
111+
}
112+
113+
int ret = 1;
114+
char buf[64];
115+
if (WaitForSingleObject(pi.hProcess, timeoutSec * 1000) != WAIT_OBJECT_0) {
116+
TerminateProcess(pi.hProcess, 1);
117+
WaitForSingleObject(pi.hProcess, 5000);
118+
snprintf(buf, sizeof(buf), "HANG (killed after %ds)", timeoutSec);
119+
} else {
120+
DWORD code = 1;
121+
GetExitCodeProcess(pi.hProcess, &code);
122+
if (code == 0) {
123+
snprintf(buf, sizeof(buf), "clean exit");
124+
ret = 0;
125+
} else {
126+
snprintf(buf, sizeof(buf), "CRASH (exit code 0x%08lX)", (unsigned long)code);
127+
}
128+
}
129+
outcome = buf;
130+
131+
CloseHandle(pi.hThread);
132+
CloseHandle(pi.hProcess);
133+
return ret;
134+
}
135+
136+
} // namespace
137+
138+
int main(int argc, char* argv[]) {
139+
if (argc >= 3 && std::strcmp(argv[1], "--child-storm") == 0)
140+
return ChildStorm(uint32_t(std::atoi(argv[2])));
141+
SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
142+
143+
uint32_t callers = defaultStormCallers;
144+
if (argc >= 2) {
145+
int n = std::atoi(argv[1]);
146+
if (n > 0) callers = uint32_t(n);
147+
}
148+
149+
int failures = 0;
150+
std::cout << "Testing process exit during an allocation storm (" << callers << " callers)"
151+
<< std::endl;
152+
for (int i = 0; i < numStormChildren; i++) {
153+
std::string outcome;
154+
failures += RunChild("--child-storm " + std::to_string(callers), stormTimeoutSec, outcome);
155+
std::cout << " storm child " << i << ": " << outcome << std::endl;
156+
}
157+
158+
if (failures) {
159+
std::cout << "CBLAS DGEMM shutdown safety test FAILED! (" << failures
160+
<< " child processes)" << std::endl;
161+
return 1;
162+
}
163+
std::cout << "CBLAS DGEMM shutdown safety test PASSED!" << std::endl;
164+
return 0;
165+
}

0 commit comments

Comments
 (0)