From 708dd4ba70f2c49d19e4082af87ef3cece45e198 Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 11 Apr 2026 18:41:32 -0600 Subject: [PATCH] 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) --- .github/workflows/ci.yml | 19 +++++++++++-------- CMakeLists.txt | 13 +++++++++++++ unittests/threading_config_test.cpp | 26 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 unittests/threading_config_test.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62ae5d24..183e1fce 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,17 +9,18 @@ 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: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DMULTITHREAD_SUPPORT_ENABLED=${{ matrix.multithread }} - name: Build run: cmake --build build -j @@ -28,17 +29,18 @@ 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: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DMULTITHREAD_SUPPORT_ENABLED=${{ matrix.multithread }} - name: Build run: cmake --build build -j @@ -85,17 +87,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 @@ -114,7 +117,7 @@ jobs: - uses: actions/checkout@v4 - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON + run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON -DMULTITHREAD_SUPPORT_ENABLED=ON - name: Build run: cmake --build build -j @@ -133,7 +136,7 @@ jobs: - uses: actions/checkout@v4 - name: Configure - run: cmake -B build -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DENABLE_THREAD_SANITIZER=ON + run: cmake -B build -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; +}