Let mock.Invocations.Clear() remove traces of earlier invocations more thoroughly
#854
Conversation
| @@ -68,6 +76,9 @@ public void Clear() | |||
| this.invocations = null; | |||
| this.count = 0; | |||
| this.capacity = 0; | |||
|
|
|||
| this.owner.Setups.UninvokeAll(); | |||
| // ^ TODO: Currently this could cause a deadlock as another lock will be taken inside this one! | |||
There was a problem hiding this comment.
This raises the question of whether Moq should generally perform more coarse-grained locking.
This would probably mean having just one lock object on the Mock instance that would be used both during setups and invocations.
Client code should probably perform all setups first, and only then start invoking the mocked object (possibly from several threads). Code that is structured that way already shouldn't be affected by a move to more coarse-grained locking.
There was a problem hiding this comment.
I had already implemented this, but when I ran some benchmarking it turned out that more coarse-grained locking appears to be slower in general than if each collection kept its own locks.
It also appears that dead-locks currently cannot happen, because we don't have any cyclic call dependencies between SetupCollection and InvocationCollection (IIRC); but that could always change in the future if these collections are modified.
For now, let's leave this as a future TODO.
stakx
force-pushed
the
issue-733
branch
2 times, most recently
from
d83cb8c
to
86416a7
Compare
Resolves #733.