Skip to content

fix(concurrency): re-enable ThreadPool tests under non-sanitizer CI #588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mvillmow wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(concurrency): re-enable ThreadPool tests under non-sanitizer CI #588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/_required.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,12 @@ jobs:
# ============================================================
# 3. integration-tests
# C++ integration/sample/example/application tests
# + all sanitizer builds (asan, ubsan, tsan, lsan) as matrix
# + all sanitizer builds (asan, ubsan, tsan, lsan) as matrix.
# The sanitizer Makefile rules and CMake presets add
# -DKEYSTONE_SANITIZER_BUILD=1 to CMAKE_CXX_FLAGS, which makes two
# ThreadPool construction/destruction tests skip themselves at runtime
# via GTEST_SKIP() (see #511, #586). Those two tests run unfiltered in
# the non-sanitizer `unit-tests` job above.
# ============================================================
integration-tests:
name: integration-tests
Expand Down
6 changes: 3 additions & 3 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Debug",
"ENABLE_COVERAGE": "OFF",
"CMAKE_CXX_FLAGS": "-fsanitize=thread"
"CMAKE_CXX_FLAGS": "-fsanitize=thread -DKEYSTONE_SANITIZER_BUILD=1"
}
},
{
Expand All @@ -65,7 +65,7 @@
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Debug",
"ENABLE_COVERAGE": "OFF",
"CMAKE_CXX_FLAGS": "-fsanitize=address,undefined"
"CMAKE_CXX_FLAGS": "-fsanitize=address,undefined -DKEYSTONE_SANITIZER_BUILD=1"
}
},
{
Expand All @@ -75,7 +75,7 @@
"cacheVariables": {
"CMAKE_BUILD_TYPE": "Debug",
"ENABLE_COVERAGE": "OFF",
"CMAKE_CXX_FLAGS": "-fsanitize=undefined"
"CMAKE_CXX_FLAGS": "-fsanitize=undefined -DKEYSTONE_SANITIZER_BUILD=1"
}
}
],
Expand Down
14 changes: 9 additions & 5 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ endif
# Compiler flags
BUILD_FLAGS_debug := -O0 -g -D_DEBUG
BUILD_FLAGS_release := -O3 -DNDEBUG
BUILD_FLAGS_asan := -fsanitize=address -fno-omit-frame-pointer
BUILD_FLAGS_ubsan := -fsanitize=undefined -fno-omit-frame-pointer
BUILD_FLAGS_lsan := -fsanitize=leak -fno-omit-frame-pointer
BUILD_FLAGS_tsan := -fsanitize=thread -fno-omit-frame-pointer
BUILD_FLAGS_msan := -fsanitize=memory -fno-omit-frame-pointer
# Issue #586: KEYSTONE_SANITIZER_BUILD gates a GTEST_SKIP() in two ThreadPool
# construction/destruction tests whose runtime exceeds CTEST_TIMEOUT under
# thread-instrumentation. The non-sanitizer unit-tests CI job runs them
# unfiltered.
BUILD_FLAGS_asan := -fsanitize=address -fno-omit-frame-pointer -DKEYSTONE_SANITIZER_BUILD=1
BUILD_FLAGS_ubsan := -fsanitize=undefined -fno-omit-frame-pointer -DKEYSTONE_SANITIZER_BUILD=1
BUILD_FLAGS_lsan := -fsanitize=leak -fno-omit-frame-pointer -DKEYSTONE_SANITIZER_BUILD=1
BUILD_FLAGS_tsan := -fsanitize=thread -fno-omit-frame-pointer -DKEYSTONE_SANITIZER_BUILD=1
BUILD_FLAGS_msan := -fsanitize=memory -fno-omit-frame-pointer -DKEYSTONE_SANITIZER_BUILD=1

BUILD_DIR ?= build
EMPTY :=
Expand Down
28 changes: 18 additions & 10 deletions tests/unit/test_thread_pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@
using namespace keystone::concurrency;

// Test: Create and destroy ThreadPool
// DISABLED under sanitizers: TSan instruments thread-startup so heavily
// that ThreadPool(4) + ~ThreadPool() takes >600s on CI runners.
// Pool construction/destruction is validated indirectly by all other tests.
// Tracked for re-enablement under non-sanitizer CI in issue #586.
TEST(ThreadPoolTest, DISABLED_CreateAndDestroy) {
// Under sanitizer builds (asan/tsan/ubsan/lsan), thread startup/join
// instrumentation makes ThreadPool(4) + ~ThreadPool() exceed CTest's timeout,
// so the test skips itself at runtime when compiled with
// -DKEYSTONE_SANITIZER_BUILD=1 (injected by the Makefile %.asan/%.ubsan/%.lsan/%.tsan
// rules and by the asan/ubsan/tsan CMake presets). The non-sanitizer
// `unit-tests` CI job runs it normally. See issues #511 and #586.
TEST(ThreadPoolTest, CreateAndDestroy) {
#ifdef KEYSTONE_SANITIZER_BUILD
GTEST_SKIP() << "Disabled under sanitizers (#586): construction+destruction "
"exceeds CTest timeout under thread instrumentation.";
#endif
ThreadPool pool(4);
EXPECT_EQ(pool.size(), 4u);
}
Expand Down Expand Up @@ -157,11 +163,13 @@ TEST(ThreadPoolTest, NoWorkAfterShutdown) {
}

// Test: Thread pool with hardware_concurrency threads
// DISABLED under sanitizers: ASan/LSan/TSan instrument thread-startup so
// heavily that ThreadPool() + ~ThreadPool() hangs on CI runners (>120s
// CTest timeout). Construction is validated indirectly by all other tests.
// Tracked for re-enablement under non-sanitizer CI in issue #586.
TEST(ThreadPoolTest, DISABLED_HardwareConcurrency) {
// Same sanitizer-skip rationale as ThreadPoolTest.CreateAndDestroy above.
// See issues #511 and #586.
TEST(ThreadPoolTest, HardwareConcurrency) {
#ifdef KEYSTONE_SANITIZER_BUILD
GTEST_SKIP() << "Disabled under sanitizers (#586): construction+destruction "
"exceeds CTest timeout under thread instrumentation.";
#endif
ThreadPool pool; // Uses std::thread::hardware_concurrency()

EXPECT_GT(pool.size(), 0);
Expand Down
Loading