Smart Pointers, Dumb Programmers

2006-11-09 • • Comments

The class I’d just written seemed to be working just fine. But when I ran my unit tests I realised I’d made a silly mistake.

Failure to Reset

The test_reset() unit test was failing. This test exercised the following function:

void
Source::reset()
{
    ....
    sink.reset();
}

Here, sink points to a Sink instance which should be reset every time the Source instance gets reset. My test showed that Sink::reset() wasn’t getting called.

Obviously it wasn’t getting called because sink is a pointer and I hadn’t dereferenced it. Once the defect had been found, the fix was trivial:

void
Source::reset()
{
    ....
    sink->reset();
}

However did it compile?

Surely this bug should have been caught at compile time? You can’t use member access syntax on a pointer. Even if the test now passed, further investigation was needed.

Further Investigation

Here’s the declaration of sink:

class Source
{
    ....
private:
    std::auto_ptr<Sink> sink;
};

Look! It’s a smart pointer, not a raw one — in this case a std::auto_ptr which owns the dynamically allocated Sink instance and deletes it when we’re done.

So, sink.reset() should compile: it calls std::auto_ptr<Sink>::reset() instead of Sink::reset() and destroys our owned sink instance prematurely. (You can view the auto_ptr documentation here.)

This is a nasty defect which might well have caused considerable trouble if it hadn’t been exposed early by a unit test.

Lessons Learned

  1. Simple tests can expose nasty bugs.
  2. Compilers don’t catch all defects.
  3. Namespaces don’t stop you calling the wrong function.
  4. Smart pointers can be subtle.
  5. Smart programmers write unit tests.
Feedback