Story of a bug that I introduced in my codebase while making it parallel.
Hopefully, I could figure it out quickly (thanks to some well placed
assert’s), but it took me a little but more time to understand what what happening.
The problem that I was working on could be summarized as follows:
- let’s say that we have a procedure (a signal processing filter for example) that knows how to operate on some data
- and now pretend that we want to re-use that procedure (because we like it and it’s well tested) but with different types of data (new measurement instrument, new data set…) that will need some specific pre- or post-processing.
Since our core routine is supposed to be well tested and doing its job right we don’t want to modify it in any way (not even by copying its code to adapt it to our new data).
A solution to this problem is to design a class that will provide a
public method that allows to call our wonderful routine and that can be subclassed (specialized) in order to take care of applying the correct pre/post-processing.
As Herb Sutter suggests here, it’s a good idea in this case to keep the public method that will be called by clients
non-virtual, while the internal pre/post-processing methods are declared as
The rationale behing is quite simple: the sequence of commands executed in the public function does not vary1, so it doesn’t have any reasons to be
This yields us with the following base class:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54
Some subclasses are used to specialize what happened inside the pre/post-processing methods, for example:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
So far, so good.
In order to spend less time glaring at my screen and waiting for a result, I decided to run several threads in parallel, each of them applying a solver object on a line of an image.
This was easy to set using the
std::thread class from C++11.
I thought I was all done (I did even think of copying my solver object so that each thread did hold its own copy to avoid conflicts between them) but… it turned out that the threads were always using solvers of the base class.
The specialized subclasses seemed to be ignored :-/
However, when I checked, I was constantly passing one pf my specialized solver classes as arguments to the main routine…
Taking some time to think about it…
Actually, I’ve provided much of the solution in the previous section:
I did even think of copying my solver object so that each thread did hold its own copy
This is precisely the mistake:
- my main routine (before dispatching the work to the threads) was inherited from my single-threaded implementation and expected a reference to an object of the base class;
- when used directly from the main routine, the virtual function mechanisms work correctly and call the specialized implementations of
- however, when copy-constructing solvers in the creation of the threads the compiler generated a call to the copy-constructor of the
Baseclass, instead of the
Derivedclass, because I was using a reference to a
Baseobject inside this routine.
Reproducing the bug
This behaviour can be reproduced using these simple functions and the associated main:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34
You can find the whole project at this gist.
Hopefully, this bug did not take me long to figure out and fix (thanks to some well placed asserts in the code). The fix was easy: instead of creating the solver instances outside the threads, I created them inside using a small factory function. Since this factory takes only 1 parameter, it did not even cripple my thread creation calls with lots of additional arguments.
- Beware of copy-constructed instances of object! They can be tricky to get right in the context of polymorphism…
- Concurrency is always harder than you thought!
- It’s good to have some debugging hooks/well placed assertions to enforce and check your assumptions… especially when moving from sequential to paralle execution ;)