12

I'm writing a thin wrapper around Dictionary that's designed to be thread-safe. As such, some locks are required and the majority of logic is around ensuring things are locked properly and accessed in a thread-safe way.

Now, I'm trying to unit test it. One big thing I'd like to unit test is the lock behavior, to ensure it's correct. However, I've never seen this done anywhere so I'm not sure how to go about it. Also, I know I could just use a bunch of threads to throw stuff at the wall, but with this type of test, there is no guarantee it'll fail when it's wrong. It's up to OS defined behavior with thread scheduling.

What ways are there to ensure that my locking behavior is correct with unit tests?

CharlesB
  • 86,532
  • 28
  • 194
  • 218
Earlz
  • 62,085
  • 98
  • 303
  • 499
  • 5
    +1 on the question's concept; curious why you aren't using a `ConcurrentDictionary` for this particular case. – Tim M. Feb 26 '13 at 19:04
  • @TimMedora portable class library targeting Windows Phone :( – Earlz Feb 26 '13 at 19:05
  • Understood. From my experience, bombarding a presumed thread-safe class with thousands of operations (cross-thread, varied sequence) will quickly reveal problems, but of course that's not deterministic. You might add a few internal test hooks to help force the scenario you want. – Tim M. Feb 26 '13 at 19:23
  • @TimMedora it could be done deterministically in a test: iterating in one thread in between signaling other thread to modify the collection and then verifying if modification not occurred and there were no exceptions thrown. – Kimi Feb 26 '13 at 19:31
  • 1
    @Kimi - I think that would be a valid test in one scenario (iteration) since you can signal thread #2 before iterating over the next item. I don't know if that same paradigm would be be deterministic in all scenarios (such as two concurrent Add() operations). – Tim M. Feb 26 '13 at 19:38
  • @TimMedora, that would require a different test and for this reason I'm advocating to precisely (deterministically) mock up the race condition and test the data integrity instead of behaviouristically testing invocation of locks. – Kimi Feb 26 '13 at 19:43
  • Perhaps of interest: http://stackoverflow.com/questions/1715822/unit-test-for-thread-safe-ness – flup Feb 27 '13 at 11:17

3 Answers3

3

Locking is just an implementation detail. You should mock up the race condition itself and test if data hasn't lost its integrity in the test.

This would be a benefit if you decide to change the implementation of your dictionary and use other synchronisation primitives.

Testing locking will prove that you are invoking a lock statement where you are expecting it. While testing with a mocked up race condition could reveal places you did not expect to synchronise.

Kimi
  • 13,621
  • 9
  • 55
  • 84
  • That's basically relying on Dictionary breaking if it's accessed concurrently – Earlz Feb 26 '13 at 19:05
  • No it should stay in a correct state, that would be proved by your test. – Kimi Feb 26 '13 at 19:09
  • 2
    Well, it's relying on implementation details of Dictionary then. I actually need to test that Dictionary is just never attempted to be accessed in a concurrent way because the whole thing basically isn't thread-safe for my purposes – Earlz Feb 26 '13 at 19:10
  • Dictionary could and should be readable concurrently, only read/write should be synchronized. And standard dictionary will expose concurrent read/write access issues you trying to avoid, that will be visible in the test. – Kimi Feb 26 '13 at 19:25
2

I basically ended up doing what @Alexei said. To be more specific, this is what I did:

  1. Make a PausingDictionary which basically just has a callback for every method, but otherwise just passes through to a regular dictionary
  2. Abstract my code(using DI, etc) so that I can use PausingDictionary instead of a regular Dictionary when testing
  3. Added two ConcurrentDictionaries in my unit test. One called "Accessed" and one called "GoAhead". The key to thiis is a combination of "action"+Thread.GetHashCode().ToString() (where action is different for each callback)
  4. Initialized everything to false and added some extension methods to make working with it a bit easier
  5. Setup the dictionary's callbacks to set Accessed for the thread to true, but it would then wait in the callback until GoAhead was true
  6. Started two threads from within the unit test. One thread would access dictionary, but because GoAhead is false for that thread, it'd sit there. The second thread would then also attempt to access the dictionary
  7. I'd have an assertion that Accessed for that thread is false, because my code should lock it out.

There's a bit more to it than that. I'd also need to mock up an IList, but I don't think I will. These unit tests, while valuable, are definitely not the easiest thing in the world to write. Aside from setup code and fake interface implementations and such, each test ends up being about 25 lines of not-boilerplate code. Locking is hard. Proving that your locking is effective is even harder. Amazingly though, this kind of pattern can allow you to test almost any scenario. But, it's very verbose and does not make for pretty tests

So, despite it being hard to write the tests, this works perfectly. When I remove a lock is consistently fails and when I add back the lock, it consistently passes.

Edit:

I think this method of "controlling interleave" of threads would also make it possible to test thread-safety, given that you write a test for each possible interleave. With some code this would be impossible, but I just want to say this is in no way limited to only locking code. You could do the same way to consistently duplicate a thread-safe failure like foo.Contains(x) and then var tmp=foo[x]

Earlz
  • 62,085
  • 98
  • 303
  • 499
1

I don't think with Dictionary itself you can achieve reliable tests - your goal is to make 2 calls to run in parallel, which is not going to happen reliably.

You can try to have custom Dictionary (i.e. derive from regular one) and add callbacks for all Get/Add methods. Than you'll be able to delay calls in 2 threads as needed... You will need separate synchronization between 2 threads to make your test code run the way you want and not deadlock all the time.

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179