Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: address second round of review feedback
- Combine CountMatches + MarkMatchingCallsVerified into single
  CountMatchingCalls(calls, markVerified) to avoid concurrent-mutation
  window between two separate passes
- Declare _singleBehavior and _behaviors as volatile to match usage
  pattern and prevent maintenance trap (removes Volatile.Read/Write)
- Expand AddBehavior comment to explain memory model guarantee: volatile
  write to _singleBehavior acts as release fence ensuring _behaviors is
  visible to lock-free readers
- Optimize AddSetup snapshot rebuild: shallow-copy existing dictionary
  and only re-array the affected member (O(N) per member, not O(total))
- Add comment on _callHistory assignment in Reset() clarifying volatile
  field semantics
  • Loading branch information
thomhurst committed Mar 30, 2026
commit 7857cae92b10b14ff9b47d726b7ed8f288196a39
14 changes: 7 additions & 7 deletions TUnit.Mocks/MockEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ public void AddSetup(MethodSetup setup)
_hasStatefulSetups = true;
}

// Rebuild lock-free snapshot for the read path
var snapshot = new Dictionary<int, MethodSetup[]>(dict.Count);
foreach (var kvp in dict)
{
snapshot[kvp.Key] = kvp.Value.ToArray();
}
// Rebuild lock-free snapshot: shallow-copy existing, only re-array the affected member
var prev = _setupsSnapshot;
var snapshot = prev is null
? new Dictionary<int, MethodSetup[]>()
: new Dictionary<int, MethodSetup[]>(prev);
snapshot[setup.MemberId] = list.ToArray();
_setupsSnapshot = snapshot;
}
}
Expand Down Expand Up @@ -524,7 +524,7 @@ public void Reset()
PendingRequiredState = null;
}

_callHistory = new ConcurrentQueue<CallRecord>();
_callHistory = new ConcurrentQueue<CallRecord>(); // volatile field — assignment is a volatile write
Volatile.Write(ref _autoTrackValues, null);
Volatile.Write(ref _eventSubscriptions, null);
Volatile.Write(ref _onSubscribeCallbacks, null);
Expand Down
21 changes: 11 additions & 10 deletions TUnit.Mocks/Setup/MethodSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ public sealed class MethodSetup
private readonly IArgumentMatcher[] _matchers;
private readonly Lock _behaviorLock = new();
/// <summary>Fast path for the common single-behavior case. Avoids list + lock on read.</summary>
private IBehavior? _singleBehavior;
private List<IBehavior>? _behaviors;
private volatile IBehavior? _singleBehavior;
private volatile List<IBehavior>? _behaviors;
private List<EventRaiseInfo>? _eventRaises;
private EventRaiseInfo[]? _eventRaisesSnapshot;
private Dictionary<int, object?>? _outRefAssignments;
Expand Down Expand Up @@ -66,20 +66,22 @@ public void AddBehavior(IBehavior behavior)
{
if (_singleBehavior is null && _behaviors is null)
{
// First behavior: store directly, no list needed
Volatile.Write(ref _singleBehavior, behavior);
_singleBehavior = behavior;
return;
}

// Promote to list on second behavior — write list before clearing single
// so concurrent lock-free readers always find at least one path
// Promote to list on second behavior. Write _behaviors before clearing
// _singleBehavior: both fields are volatile, so the volatile write to
// _singleBehavior acts as a release fence, guaranteeing that a lock-free
// reader in GetNextBehavior that sees _singleBehavior == null will also
// see the updated _behaviors reference.
if (_behaviors is null)
{
_behaviors = [_singleBehavior!];
}

_behaviors.Add(behavior);
Volatile.Write(ref _singleBehavior, null);
_singleBehavior = null;
}
}

Expand Down Expand Up @@ -187,13 +189,12 @@ public string[] GetMatcherDescriptions()
public IBehavior? GetNextBehavior()
{
// Fast path: single behavior (most common case — no lock needed)
var single = Volatile.Read(ref _singleBehavior);
if (single is not null)
if (_singleBehavior is { } single)
{
return single;
}

if (Volatile.Read(ref _behaviors) is null)
if (_behaviors is null)
{
return null;
}
Expand Down
28 changes: 8 additions & 20 deletions TUnit.Mocks/Verification/CallVerificationBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void WasCalled(Times times, string? message)

var allCallsForMember = _engine.GetCallsFor(_memberId);

var matchingCount = CountMatches(allCallsForMember);
var matchingCount = CountMatchingCalls(allCallsForMember, markVerified: false);

if (!times.Matches(matchingCount))
{
Expand All @@ -62,7 +62,7 @@ public void WasCalled(Times times, string? message)
}

// Mark matched calls as verified only after assertion passes
MarkMatchingCallsVerified(allCallsForMember);
CountMatchingCalls(allCallsForMember, markVerified: true);
}

/// <inheritdoc />
Expand All @@ -77,35 +77,23 @@ public void WasCalled(Times times, string? message)
/// <inheritdoc />
public void WasCalled(string? message) => WasCalled(Times.AtLeastOnce, message);

private int CountMatches(IReadOnlyList<CallRecord> calls)
private int CountMatchingCalls(IReadOnlyList<CallRecord> calls, bool markVerified)
{
if (_matchers.Length == 0)
{
return calls.Count;
}

var count = 0;
for (int i = 0; i < calls.Count; i++)
{
if (MatchesArguments(calls[i].Arguments))
if (_matchers.Length == 0 || MatchesArguments(calls[i].Arguments))
{
count++;
if (markVerified)
{
calls[i].IsVerified = true;
}
}
}
return count;
}

private void MarkMatchingCallsVerified(IReadOnlyList<CallRecord> calls)
{
for (int i = 0; i < calls.Count; i++)
{
if (_matchers.Length == 0 || MatchesArguments(calls[i].Arguments))
{
calls[i].IsVerified = true;
}
}
}

private bool MatchesArguments(object?[] arguments)
{
if (_matchers.Length != arguments.Length)
Expand Down
Loading