From 4804fb6e036dc325f56eecde8e875717a357e187 Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 11 Apr 2026 20:25:46 -0600 Subject: [PATCH] Fix #666: TSAN builds don't actually use threads (#670) * Fix #666: TSAN builds don't actually use threads TSAN builds did not explicitly enable MULTITHREAD_SUPPORT_ENABLED, so they relied on the CMake default. This adds -DMULTITHREAD_SUPPORT_ENABLED=ON to both TSAN jobs and expands the CI matrix to cover threading on/off with Debug/Release for Linux, macOS, and Windows MSVC. Also excludes async-dependent .chai tests from non-threaded builds and adds a Threading_Config_Test to verify the threading configuration is correct in both modes. Co-Authored-By: Claude Opus 4.6 (1M context) * Address review: use Ninja generator for all unix CI targets Requested by @lefticus in PR #670 review. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: leftibot Co-authored-by: Claude Opus 4.6 (1M context) --- .github/workflows/ci.yml | 41 ++++++++++++++++++++++------- CMakeLists.txt | 13 +++++++++ unittests/threading_config_test.cpp | 26 ++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 unittests/threading_config_test.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62ae5d24..ff0c6778 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,17 +9,21 @@ on: jobs: linux: - name: Linux GCC ${{ matrix.build_type }} + name: Linux GCC ${{ matrix.build_type }} threads=${{ matrix.multithread }} runs-on: ubuntu-latest strategy: fail-fast: false matrix: build_type: [Debug, Release] + multithread: [ON, OFF] steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: sudo apt-get install -y ninja-build + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DMULTITHREAD_SUPPORT_ENABLED=${{ matrix.multithread }} - name: Build run: cmake --build build -j @@ -28,17 +32,21 @@ jobs: run: ctest --test-dir build --output-on-failure macos: - name: macOS AppleClang ${{ matrix.build_type }} + name: macOS AppleClang ${{ matrix.build_type }} threads=${{ matrix.multithread }} runs-on: macos-latest strategy: fail-fast: false matrix: build_type: [Debug, Release] + multithread: [ON, OFF] steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: brew install ninja + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DMULTITHREAD_SUPPORT_ENABLED=${{ matrix.multithread }} - name: Build run: cmake --build build -j @@ -56,8 +64,11 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: sudo apt-get install -y ninja-build + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON - name: Build run: cmake --build build -j @@ -75,8 +86,11 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: brew install ninja + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_ADDRESS_SANITIZER=ON -DENABLE_UNDEFINED_SANITIZER=ON - name: Build run: cmake --build build -j @@ -85,17 +99,18 @@ jobs: run: ctest --test-dir build --output-on-failure windows: - name: Windows MSVC ${{ matrix.build_type }} + name: Windows MSVC ${{ matrix.build_type }} threads=${{ matrix.multithread }} runs-on: windows-latest strategy: fail-fast: false matrix: build_type: [Debug, Release] + multithread: [ON, OFF] steps: - uses: actions/checkout@v4 - name: Configure - run: cmake -B build + run: cmake -B build -DMULTITHREAD_SUPPORT_ENABLED=${{ matrix.multithread }} - name: Build run: cmake --build build --config ${{ matrix.build_type }} -j @@ -113,8 +128,11 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: sudo apt-get install -y ninja-build + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON -DMULTITHREAD_SUPPORT_ENABLED=ON - name: Build run: cmake --build build -j @@ -132,8 +150,11 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Install Ninja + run: brew install ninja + - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON + run: cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON -DMULTITHREAD_SUPPORT_ENABLED=ON - name: Build run: cmake --build build -j diff --git a/CMakeLists.txt b/CMakeLists.txt index 7fcb4bc2..4a4b6f55 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -292,6 +292,15 @@ endif() file(GLOB UNIT_TESTS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/unittests/ ${CMAKE_CURRENT_SOURCE_DIR}/unittests/*.chai ${CMAKE_CURRENT_SOURCE_DIR}/unittests/3.x/*.chai) list(SORT UNIT_TESTS) +if(NOT MULTITHREAD_SUPPORT_ENABLED) + list(REMOVE_ITEM UNIT_TESTS + async_engine_lifetime.chai + future.chai + list_push_front.chai + move_async.chai + ) +endif() + file(GLOB PERFORMANCE_TESTS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/performance_tests/ ${CMAKE_CURRENT_SOURCE_DIR}/performance_tests/*.chai) list(SORT PERFORMANCE_TESTS) @@ -430,6 +439,10 @@ if(BUILD_TESTING) target_link_libraries(emscripten_eval_test ${LIBS}) add_test(NAME Emscripten_Eval_Test COMMAND emscripten_eval_test) + add_executable(threading_config_test unittests/threading_config_test.cpp) + target_link_libraries(threading_config_test ${LIBS}) + add_test(NAME Threading_Config_Test COMMAND threading_config_test) + install(TARGETS test_module RUNTIME DESTINATION bin LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}/chaiscript") endif() endif() diff --git a/unittests/threading_config_test.cpp b/unittests/threading_config_test.cpp new file mode 100644 index 00000000..7696d13a --- /dev/null +++ b/unittests/threading_config_test.cpp @@ -0,0 +1,26 @@ +#include + +int main() { +#ifndef CHAISCRIPT_NO_THREADS + // When threading is enabled, verify that async() is registered and functional + chaiscript::ChaiScript chai; + const auto result = chai.eval(R"( + var f = async(fun() { return 42; }); + f.get(); + )"); + if (result != 42) { + return EXIT_FAILURE; + } +#else + // When threading is disabled, verify that async() is NOT registered + chaiscript::ChaiScript chai; + try { + chai.eval("async(fun() { return 0; })"); + return EXIT_FAILURE; + } catch (const std::exception &) { + // Expected: async should not exist + } +#endif + + return EXIT_SUCCESS; +}