Practical Testing: 37 - Bringing the code up to date
Previously on “Practical Testing”… Having just resurrected this series of blog posts about testing a non-trivial piece of real-world C++ code I’ve fixed a few bugs and done a bit of refactoring. There’s one more step required to bring the code in this article right up to date.
The timer queue that is the focus of these blog posts is part of The Server Framework. This body of code has been around since 2001 and has evolved to support new platforms and compilers. One of the things that I do from time to time is remove support for old platforms and compilers. This allows me to start using exciting new C++ features (around 10 years after they were first ’new’) and it means that code that is present just to work around issues in old platforms can be removed. The timer queue has quite a bit of code that needn’t be there now that Windows XP has passed away.
In 2008, in episode 17, I added support for
GetTickCount64() which removed some of the complexity from timer management. We kept the old version around as you needed to have Windows Vista or later to use that API call. Now that I no longer support Windows XP every supported platform has
GetTickCount64() and there’s no reason to support the XP version. Removing that removes quite a bit of complexity from both the internals and the public API; you no longer have to decide which version to use as there is only one!
As the recent bug fixes have shown, there are also two versions of timer dispatch. The original version which holds a lock whilst calling into user code during timer dispatch and the improved version that doesn’t. Holding locks whilst calling back into user code via callback interfaces is a surefire way to create lock inversions and deadlock. The old dispatch method was originally kept because I was unsure of the performance characteristics of the new way. The improved version was added in 2008 in episode 18 and every piece of development that I’ve done since has used the new dispatch method. The performance difference, if present, is not relevant and the removal of the ability for code to take part in lock inversions is important for library code. So the original method of dispatch should be removed.
The removal of this functionality massively simplifies the code and the tests. 60 tests can be removed which is almost a third. Most of the code that was moved into the common base class for the two versions of the queues can be moved into the ‘Ex’ version with
GetTickCount64() support. I expect that I should, eventually, rename the
CCallbackTimerQueueEx class to
CallbackTimerQueue, but not yet. There are some systems that use this class explicitly rather than via
CThreadedCallbackTimerQueue and the enum that we’re removing in this release. It will be easier to introduce another breaking change in a future release as it means that fixing the breaking changes due to this release is slightly easier and more obvious and the fix when I do change the name will be a simple rename in the code that uses the old class…
The code is here on GitHub and new rules apply.