From 37e79dcdd5eb1cab0ae2c02d9fabdc40aeec769a Mon Sep 17 00:00:00 2001 From: Pranit Date: Thu, 22 Oct 2026 00:48:56 -0400 Subject: [PATCH] Add a nightly ThreadSanitizer run of the unit tests --- .github/workflows/tsan.yml | 78 ++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 13 ++++++- build.zig | 9 +++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/tsan.yml diff --git a/.github/workflows/tsan.yml b/.github/workflows/tsan.yml new file mode 100644 index 000000000..34fb2d485 --- /dev/null +++ b/.github/workflows/tsan.yml @@ -0,0 +1,78 @@ +name: Thread Sanitizer + +# Nightly unit tests built with ThreadSanitizer. The job reports races and +# never fails because of them: it lists each distinct race in the job summary +# and uploads the full log. A pull request that changes this workflow runs it +# once, so a change that breaks the nightly run shows before it merges. + +on: + schedule: + - cron: "47 7 * * *" + pull_request: + paths: + - .github/workflows/tsan.yml + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: tsan-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + tsan: + name: Unit tests with ThreadSanitizer + runs-on: ubuntu-latest + timeout-minutes: 90 + steps: + - uses: actions/checkout@v4 + + # Restores main's unit-test cache for the dependencies, compiler_rt, and + # libc it keeps. This job never saves a cache. + - name: Setup Zig + uses: ./.github/actions/setup-zig + with: + version: "0.17.0" + cache-key: ci-unit-ReleaseSafe + + - name: Run the unit tests + env: + # Report every race, keep running, and leave the exit status to the + # tests themselves. + TSAN_OPTIONS: halt_on_error=0:abort_on_error=0:exitcode=0 + run: | + set -o pipefail + zig build test -Dsanitize-thread=true --summary all 2>&1 | tee tsan.log + continue-on-error: true + + - name: Summarize races + if: always() + run: | + { + echo "## ThreadSanitizer" + echo + count=$(grep -c 'WARNING: ThreadSanitizer' tsan.log || true) + echo "Reports: ${count:-0}" + echo + if [ "${count:-0}" -gt 0 ]; then + echo "| Reports | Race |" + echo "|---|---|" + grep 'SUMMARY: ThreadSanitizer' tsan.log \ + | sed -E 's/^.*SUMMARY: ThreadSanitizer: //; s/ \([^)]*\)//' \ + | sort | uniq -c | sort -rn \ + | awk '{ n = $1; $1 = ""; sub(/^ /, ""); printf "| %s | `%s` |\n", n, $0 }' + fi + echo + echo '```' + grep -E '^Build Summary' tsan.log || echo "no build summary" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload the log + if: always() + uses: actions/upload-artifact@v4 + with: + name: tsan-log + path: tsan.log + if-no-files-found: ignore diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d68c3521..94d71f336 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -113,7 +113,17 @@ The Fuzz workflow fuzzes each target on `main` every night for about 20 minutes, A saved input holds the values the test reads from its `std.testing.Smith`, in order. For example, a test that calls `smith.slice` reads a 4-byte little-endian length and then that many bytes. Inputs that the fuzzer saves are already in this form. -To add a target, write the fuzz test in the file it covers and add an entry to `tests/fuzz/targets.json`. For a target outside the session manager, also import its file in `src/fuzz_targets.zig`. Pass the saved inputs to the test with `.corpus = &(.{ } ++ @import("fuzz_corpus")..*)`, or with `@import("build_options").fuzz_corpus_` in the session manager. Set `iterations` so that one round of `--fuzz` takes about 30 seconds on a developer machine. `scripts/tests/test_fuzz_nightly.py` fails when an entry names a test that doesn't exist or that `zig build fuzz` can't reach. +To add a target, write the fuzz test in the file it covers and add an entry to `tests/fuzz/targets.json`. For a target outside the session manager, also import its file in `src/fuzz_targets.zig`. Pass the saved inputs to the test with `.corpus = &(.{ } ++ @import("fuzz_corpus")..*)`, or with `@import("build_options").fuzz_corpus_` in the session manager. Set `iterations` so that one round of `--fuzz` takes about 30 seconds on a developer machine. `scripts/tests/test_fuzz_nightly.py` fails when an entry names a test that doesn't exist or that `zig build fuzz` can't reach. The MCP engine in `src/mcp/` imports only the standard library, so its targets live in `src/core/mcp_host/wire_tests.zig`. A file that holds only tests, like that one, is also imported from the test block in `src/main.zig`. + +### Find data races + +`-Dsanitize-thread=true` builds fx and its tests with ThreadSanitizer, which reports two threads touching the same memory without synchronization. The build is slow and large, so install it to its own prefix: + +```bash +TSAN_OPTIONS=halt_on_error=0:abort_on_error=0 zig build test -Dsanitize-thread=true -p /tmp/fx-tsan +``` + +Without those options, the sanitizer aborts a test binary once it has reported a race, so that binary's later tests don't run. The Thread Sanitizer workflow runs the unit tests this way on `main` every night and lists each race in its job summary. It reports races without failing. ### Check startup performance @@ -215,6 +225,7 @@ Other runs start only on request: | Long-turn memory benchmark | `gh workflow run bench.yml --ref your-branch -f long_turn_memory=true` | | libfx runtime benchmark | `gh workflow run bench.yml --ref your-branch -f libfx_runtime=true` | | Fuzzing of one target | `gh workflow run fuzz.yml --ref your-branch -f target=` | +| Unit tests with ThreadSanitizer | `gh workflow run tsan.yml --ref your-branch` | Only maintainers can start these runs or add the `ci:macos` label. If you're an outside contributor and your change needs one of them, ask in the pull request and a maintainer will start it. If CI doesn't start at all on your first pull request, a maintainer may need to approve the workflow run. diff --git a/build.zig b/build.zig index 1d2756097..862286b8f 100644 --- a/build.zig +++ b/build.zig @@ -39,6 +39,12 @@ pub fn build(b: *std.Build) void { "Build a Node-API addon surface (core)", ) orelse .none; + const sanitize_thread = b.option( + bool, + "sanitize-thread", + "Build fx and its tests with ThreadSanitizer", + ) orelse false; + const git_commit = readGitCommit(b); const app_version = readAppVersion(b); const update_channel = b.option(UpdateChannel, "update-channel", "Build update channel (stable or dev)") orelse .stable; @@ -68,11 +74,13 @@ pub fn build(b: *std.Build) void { .unwind_tables = .none, .error_tracing = false, .strip = optimize != .debug, + .sanitize_thread = sanitize_thread, }), }); exe.root_module.addImport("build_options", build_options.createModule()); exe.root_module.addImport("fuzz_corpus", fuzz_corpus); const session_manager = addSessionManager(b, exe.root_module, target, optimize, null, fuzz_corpora); + session_manager.sanitize_thread = sanitize_thread; b.installArtifact(exe); @@ -104,6 +112,7 @@ pub fn build(b: *std.Build) void { const session_manager_hook_tests = b.addTest(.{ .root_module = sessionManagerModule(b, target, optimize, null, true, fuzz_corpora), }); + session_manager_hook_tests.root_module.sanitize_thread = sanitize_thread; const run_session_manager_hook_tests = b.addRunArtifact(session_manager_hook_tests); const session_manager_test_step = b.step("test-session-manager", "Run the session manager's tests"); session_manager_test_step.dependOn(&run_session_manager_tests.step);