Practical Testing: 12 - Threading is orthogonal

Previously, on Practical Testing: we finished our reworking on the code we were left with a simpler timer queue that passed all of our tests, but was single threaded and needed manual intervention from client code to manage timeouts. Today we'll add back the multi-threaded functionality so that the queue manages timeouts automatically.

Our original implementation of the callback timer queue used a thread to manage the timeouts. The threading was integral to the timer queue code and this made testing the queue logic difficult because, to test the queue, we had to control the thread. The thread simply looped; working out when the next timer was due, waiting until that time and then processing the timer. When we redeveloped the timer queue we recognised that the threading was orthogonal to the main work of the timer queue. In fact, if we built the queue without threading we could add any sort of polling to the queue; though a self contained threaded queue was certainly desirable, the new code would allow us to build a queue that used an existing thread to manage the timers, or one that used some other stimuli for triggering the queue polling. The non-threaded queue was also considerably easier to test.

I've found that TDD tends to focus the mind on the cohesion of a particular object. Loosely coupled, highly cohesive objects are easier to develop and easier to write tests for. TDD forces you to think of the separation of concerns up front because failing to do so seems to result in complex, tightly coupled objects with low cohesion that try to do too much and are impossible to test.

It's all very well to separate out the threading but if you need it, you need it. Today we'll write a threaded callback timer queue that builds on our existing work.

The requirements for the threaded timer queue are that it will manage a thread that polls the timer queue automatically. You create the object; it fires up its worker thread and runs the timer queue for you until you destroy the object. The important thing to realise, from the point of view of our existing code, is that suddenly we have multiple threads calling into the timer queue object. Our timer queue does nothing to protect itself from these multiple threads and, as it stands, is not safe for this kind of usage. It's easy to make the timer queue safe for use by multiple threads; it just involves adding a critical section and locking around the queue's data structure accesses so that one thread can't be adding a new timer when another thread is processing the timer collection. The question is, should the existing timer queue do the locking or should the users of the queue do the locking; in this instance, the user of the queue is the threaded timer queue object. There's a fairly good argument for adding the locking to the existing object, after all, it's more likely that we'll need a thread safe timer queue than we'll perform all operations on the queue from a single thread. However, adding the locking to the existing queue makes the queue ever so slightly less friendly to reuse; there's a chance that we might not want the overhead of the locking and if we place it in the timer queue object then we have to have it. Also the locking granularity might not be quite right for the users of the class; if that's the case then the user will need its own lock and the lock inside t he timer queue is redundant. Given these, small, issues I've decided to keep the locking outside of the timer queue object and provide it in the threaded object instead. If we later find that we really would prefer the locking in the object itself we could always derive from it and add the locking in the derived class...

So, our threaded timer queue will provide a thread that polls the queue and actions expired timers and it will provide locking so that the underlying queue can be used safely from multiple threads. The class could look something like this:

class CThreadedCallbackTimerQueue : 
   private CThread, 
   private CCallbackTimerQueue
   public :
      explicit CThreadedCallbackTimerQueue(
         const DWORD maxTimeout);
      explicit CThreadedCallbackTimerQueue(
         const IProvideTickCount &tickProvider);
         const DWORD maxTimeout,
         const IProvideTickCount &tickProvider);
      Handle SetTimer(
         Timer &timer,
         const DWORD timeoutMillis,
         const UserData userData);
      bool CancelTimer(
         Handle handle);
   private :
      DWORD GetNextTimeout();
      void InitiateShutdown();
      void SignalStateChange();
      // Implement CThread
      virtual int Run();
      mutable CCriticalSection m_criticalSection;
      CAutoResetEvent m_stateChangeEvent;
      volatile bool m_shutdown;
      // No copies do not implement
      CThreadedCallbackTimerQueue(const CThreadedCallbackTimerQueue &rhs);
      CThreadedCallbackTimerQueue &operator=(const CThreadedCallbackTimerQueue &rhs);

I've chosen to derive from CCallbackTimerQueue, albeit privately, rather than simply to contain it because it allows us to use the nested types that it defines, Handle and Timer without further clarification. Composing the threaded queue using containment would mean that our timer functions would need to be declared as using CCallbackTimerQueue::Handle, etc, which seems, to me, messier. Of course we could define our own types, based on the ones in our contained object, but that also seems wrong in this instance...

Our threaded timer queue puts together a thread, a timer queue, a critical section and an auto reset event. The event is used by the thread that polls the queue;

int CThreadedCallbackTimerQueue::Run()
   while (!m_shutdown)
      DWORD timeout = GetNextTimeout();
      if (timeout == 0)
         CCriticalSection::Owner lock(m_criticalSection);
   return 0;

We wait on the event, rather than sleeping, because we need to wake the worker thread when the state of the timer queue changes. If the queue is empty the thread will wait for an INFINITE length of time; adding a timer will cause the thread to wake and recheck the next timeout and then wait for as long as the timer requires.

We manage the event in our timer functions; these wrap the underlying CCallbackTimerQueue functions and provide the locking and event notification that our threaded queue requires:

CThreadedCallbackTimerQueue::Handle CThreadedCallbackTimerQueue::SetTimer(
   Timer &timer,
   const DWORD timeoutMillis,
   const UserData userData)
   CCriticalSection::Owner lock(m_criticalSection);
   Handle handle = CCallbackTimerQueue::SetTimer(timer, timeoutMillis, userData);
   return handle;

The rest of the class is similar.

Testing the threaded version is considerably easier than testing our original threaded timer queue. We don't need to test the timer queue functionality in a multi-threaded environment because we can test it without the complication of threads. The only thing that we need to test for in the threaded class is that the thread does its work - we'll be pragmatic and assume that the critical section works; we could try and contrive tests that would fail if the locking wasn't present, but, well, I have a life to live...

So, pretty much the only test we really need for the threaded timer queue is this:

void CThreadedCallbackTimerQueueTest::TestMultipleTimers()
   const _tstring functionName = _T("CThreadedCallbackTimerQueueTest::TestMultipleTimers");
   Output(functionName + _T(" - start"));
   CThreadedCallbackTimerQueue timerQueue;
   CLoggingCallbackTimer timer;
   timerQueue.SetTimer(timer, 500, 1);
   timerQueue.SetTimer(timer, 1500, 2);
   timerQueue.SetTimer(timer, 300, 3);
   timer.CheckResult(_T("|OnTimer: 3|OnTimer: 1|"));
   timerQueue.SetTimer(timer, 800, 4);
   timerQueue.SetTimer(timer, 200, 5);
   timerQueue.SetTimer(timer, 2000, 6);
   timer.CheckResult(_T("|OnTimer: 5|OnTimer: 2|OnTimer: 4|"));
   timer.CheckResult(_T("|OnTimer: 6|"));
   Output(functionName + _T(" - stop"));

Yes, it's not 100% reliable because we're relying on some strategically placed sleeps to make sure it works, but since we're only really testing the automatic nature of the queue processing it's probably good enough. The complex stuff is tested independently in the tests for the non-threaded object.

Now we have code that does the same job as our original code but is simpler and easier to test. In the next episode we'll integrate this new code into a real application; and in doing so find that we're lacking some functionality that the original version possessed and that's pretty fundamental to our standard usage pattern. We'll then add a new test and reintroduce the missing functionality.

Code is here. Same rules as before. Bonus points for anyone who can tell me what we're missing before I write the next instalment.

Leave a comment