Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • `_sessionTestCount` was initialized with direct assignment (`_sessionTestCount = contexts.Count`) but decremented with `Interlocked.Decrement`
  • This is a data race: the non-atomic write could be torn or not visible to other threads performing the decrement
  • Fix: use `Interlocked.Exchange` for initialization to ensure atomic write with proper memory ordering
  • Prevents potential issues with "last test in session" events firing incorrectly

Test plan

  • Verify session-level events (first/last test) fire correctly
  • Run full engine test suite

🤖 Generated with Claude Code

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

This PR correctly addresses a thread-safety issue by using Interlocked.Exchange for initialization, which properly pairs with the existing Interlocked.Decrement operations on the _sessionTestCount field. The change ensures atomic writes with proper memory barriers, guaranteeing cross-thread visibility. The added documentation comment clearly explains the threading concerns. This is a focused, correct fix that adheres to all project guidelines.

@thomhurst thomhurst enabled auto-merge (squash) February 6, 2026 08:49
_sessionTestCount was initialized with direct assignment but decremented
with Interlocked.Decrement, creating a data race that could cause
"last test in session" events to fire incorrectly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thomhurst thomhurst force-pushed the fix/session-test-count-thread-safety branch from 5bd8b72 to 376a3f8 Compare February 6, 2026 09:39
@thomhurst thomhurst merged commit 2c31ef7 into main Feb 6, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/session-test-count-thread-safety branch February 6, 2026 10:19
This was referenced Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant