Skip to content

Fix thread-safety crash in Matcher under parallel test execution#138

Closed
fortmarek wants to merge 2 commits intoKolos65:mainfrom
fortmarek:fix/thread-safe-matcher
Closed

Fix thread-safety crash in Matcher under parallel test execution#138
fortmarek wants to merge 2 commits intoKolos65:mainfrom
fortmarek:fix/thread-safe-matcher

Conversation

@fortmarek
Copy link
Copy Markdown
Contributor

@fortmarek fortmarek commented Feb 11, 2026

Summary

  • Add NSLock to synchronize concurrent access to the Matcher singleton's matchers array

Problem

When using Swift Testing (which runs test init() methods in parallel), the Matcher singleton crashes due to unsynchronized concurrent access to its matchers: [MatcherType] array.

We observed two distinct crash sites in CI across hundreds of test runs in the tuist/tuist repository:

Crash 1: Matcher.register<A>(_:)

Multiple test structs call Matcher.register() in their init() method. Under parallel execution, concurrent append() calls to the shared matchers array cause a data race:

Crash: xctest at Matcher.register<A>(_:)

The crash manifests as a process-level crash (line_number: 0, path: "") that kills all tests in the xctest worker process. All tests assigned to that process report as failed with 0.000 second duration.

Crash 2: closure #1 in Parameter.eraseToGenericValue()

For mocked generic methods like readValue<Value>(), the macro type-erases parameters via eraseToGenericValue(). The erasure closure captures the concrete type and later calls Matcher.comparator(for: Value.self). Since the generic context has no Equatable constraint, this resolves at compile time to the non-Equatable overload, which reads the matchers array.

Full crash chain from CI:

Thread 6 Crashed::  Dispatch queue: com.apple.root.default-qos.cooperative
0   libswiftCore.dylib    _assertionFailure(_:_:file:line:flags:) + 176
1   TuistKitTests         closure #1 in Parameter.eraseToGenericValue() + 4088
2   TuistKitTests         closure #1 in Matcher.registerCustomTypes() + 116
5   TuistKitTests         Parameter.match(_:using:) + 1436
6   TuistKitTests         Parameter.match(_:) + 116
7   TuistKitTests         MockUserInputReading.Member.match(_:) + 1516
18  TuistKitTests         Mocker.mock<A>(_:_:_:) + 324
20  TuistKitTests         MockUserInputReading.readValue<A>(...) + 488

The crash is fatalError(noComparatorMessage)Matcher.comparator(for: Value.self) returns nil because the matchers array is in a corrupted state from a concurrent write on another thread.

Root cause

The matchers array is a plain [MatcherType] with no synchronization:

private var matchers: [MatcherType] = []

// Write path (from register):
matchers.append((mirror, match as Any))  // NOT thread-safe

// Read path (from comparator):
matchers.reversed().first { ... }  // NOT thread-safe

Concurrent reads and writes to a Swift Array are undefined behavior and cause crashes.

Fix

Add an NSLock to synchronize all matchers array accesses. The lock is applied at the leaf level — directly around append() calls and as a snapshot-copy before iteration — to avoid re-entrancy in the Sequence comparator path (comparator(by:)comparator(for: T.Element.self)comparator(by:)).

private let lock = NSLock()

// Write: lock around append
lock.lock()
matchers.append((mirror, match as Any))
lock.unlock()

// Read: snapshot under lock, iterate outside
lock.lock()
let snapshot = matchers
lock.unlock()
return snapshot.reversed().first { ... }?.comparator

Test plan

  • All 75 existing Mockable tests pass
  • Build succeeds on macOS

🤖 Generated with Claude Code

The `Matcher` singleton's `matchers` array is accessed without
synchronization. Under Swift Testing's parallel execution, concurrent
`register()` writes and `comparator()` reads from multiple test
`init()` methods cause data races that crash the test process.

Observed crash sites:
- `Matcher.register<A>(_:)` — concurrent appends to the array
- `closure Kolos65#1 in Parameter.eraseToGenericValue()` — concurrent reads
  via `Matcher.comparator(for:)` while another thread appends

Add an NSLock to synchronize all matchers array access. The lock is
applied at the leaf level (append and snapshot-read) to avoid
re-entrancy in the Sequence comparator path which calls
`comparator(by:)` → `comparator(for: T.Element.self)` →
`comparator(by:)`.
Replace manual NSLock usage with the existing LockedValue wrapper
to make the synchronization more robust and harder to regress.
LockedValue already uses NSRecursiveLock which handles the
re-entrant comparator lookup path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fortmarek fortmarek force-pushed the fix/thread-safe-matcher branch from 3516883 to 045a9ed Compare February 12, 2026 09:01
@fortmarek
Copy link
Copy Markdown
Contributor Author

Note on matchers.withValue { \$0 }: This is the standard thread-safe snapshot pattern (same as how e.g. ThreadSafe.value is implemented internally). The .value property accessor on LockedValue would be cleaner, but it's restricted to where Value: Sendable — and MatcherType contains Any which is not Sendable, so withValue { \$0 } is the correct way to get a snapshot here.

@fortmarek fortmarek marked this pull request as ready for review February 12, 2026 09:04
@Kolos65 Kolos65 closed this Feb 12, 2026
@Kolos65
Copy link
Copy Markdown
Owner

Kolos65 commented Feb 12, 2026

Merging in #139

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.

2 participants