-
-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Making the cache dictionary thread safe #153
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a thread-safe dictionary is a start. I also don't see any simple read-then-write patterns. It seems to only (1) read everything xor (2) write one or two items, not both.
This PR attains the basic assumptions of thread safety, as far as I can tell. However, if a caller tries to setup multiple tests simultaneously, this will lead to a race condition. This seems intuitive to the user, so I don't think we should worry about it.
One concern - I haven't checked the docs, but is ToDictionary
threadsafe? I'm pretty sure it's an extension method, but maybe the enumerator returned by the threadsafe dictionary is also threadsafe. One can hope but I'd rather verify. Code comments help (e.g. explicitly state why you think an operation is threadsafe, it helps future code readers be comfortable with it).
You are correct on all counts. I do also agree that we should not try to handle the case where a single AutoMocker instance is shared across multiple tests. IMO this leads to tests that are difficult to maintain with mock objects containing multiple setups. However, with the case that was originally brought up, was passing off the |
Fixes #149