More cow_ptr<type> Ruminations

In my previous article I looked at the cow_ptr<> copy-on-write class. There are still some interesting points remaining, and a couple of bugs to own up to. Here's what we've got.

template<typename type>
class cow_ptr // moo
{
public: // create/destroy

  explicit cow_ptr(type * p = 0);
  cow_ptr(const cow_ptr & other);
  ~cow_ptr() throw();

public: // assignment

  cow_ptr & operator=(const cow_ptr & rhs);
  void clear(); 
  ... ... ...
private: // plumbing

  cow_ptr(type * ptr, size_t * count);
  void copy();
  ... ... ...
private: // state

  type * ptr;
  size_t * count; 
};

template<typename type>
cow_ptr<type>::cow_ptr(type * p)
{
  auto_ptr<type> safe(p);
  count = new size_t(1);
  ptr = safe.release();
}

template<typename type>
cow_ptr<type>::cow_ptr(const cow_ptr & other)
    : ptr(other.ptr)
    , count(other.count)
{
  ++*count;
}

template<typename type>
cow_ptr<type>::~cow_ptr() throw()
{
  clear();
}

template<typename type>
cow_ptr<type> & cow_ptr<type>::operator=(const cow_ptr & rhs)
{
  cow_ptr old_self(ptr, count);
  ptr = rhs.ptr;
  count = rhs.count;
  ++*count;
  return *this;
}

template<typename type>
void cow_ptr<type>::clear()
{
  if (--*count == 0)
{
  delete count;
  delete ptr;

  count = 0; 
  ptr = 0;
}
}

There's an obvious bug in clear(). Both pointers should be reset to null irrespective of the count. Here's a first attempt at a fix: template<typename type>

void cow_ptr<type>::clear()
{
  if (--*count == 0)
  {
    delete count;
    delete ptr;
  }
  count = 0;
  ptr = 0;
}

This seems fine. But what if we call clear() twice in succession? That will dereference a null pointer. Not good. Looking closer, we have two delete expressions. Can either of them throw an exception? Well, delete count can't since count is a pointer to a built in type, but delete ptr can^[1]. This is a good reason for doing delete ptr after delete count. If delete ptr does throw an exception what will happen to the cow_ptr object? Both its pointers will be unchanged, but both will be invalid. Not good. We want both pointers to be set to null, even if delete ptr throws an exception:

template<typename type>
void cow_ptr<type>::clear()
{
  size_t * old_count = count;
  type * old_ptr = ptr;

  count = 0;
  ptr = 0;

  if (old_count && --*old_count == 0)
  {
    delete old_count;
    delete old_ptr;
  }
}

Bugs are gregarious creatures. They usually roam about in small swarms. If you find a bug it's a good bet it will have a few mates hiding close by...

template<typename type>
cow_ptr<type> & cow_ptr<type>::
operator=(const cow_ptr & rhs)
{
  cow_ptr old_self(ptr, count); 
  ptr = rhs.ptr;
  count = rhs.count;
  ++*count;
  return *this;
}

The problem is in ++*count; count could be a null pointer. And of course this problem also crops up in the copy constructor. And in copy()...

template<typename type>
void cow_ptr<type>::copy()
{
  if (*count > 1)
  {
    cow_ptr unshared(new type(*ptr));
    *this = unshared;
  }
}

And in fact it's even worse in copy() because ptr could be null too. If ptr is null then copy should be a no-op:

template<typename type>
void cow_ptr<type>::copy()
{
  if (ptr && count && *count > 1)
  {
    cow_ptr unshared(new type(*ptr));
    *this = unshared;
  }
}

I told you they were gregarious. Note that you can implement the copy constructor by calling the assignment operator:

template<typename type>
cow_ptr<type>::cow_ptr(const cow_ptr & other)
    : ptr(0)
    , count(0)
{
  *this = other;
}

template<typename type>
cow_ptr<type> & cow_ptr<type>::operator=(const cow_ptr & rhs)
{
  if (ptr != rhs.ptr)
  {
    cow_ptr old_self(ptr, count); 
    ptr = rhs.ptr;
    count = rhs.count;
    if (count) ++*count;
  }
  return *this;
}

It's noticeable now that "all roads lead to operator=". The copy constructor calls it. And copy() too. It's common to find one method is the primitive for many others. And you often have a choice of which one to make the primitive. For example we could fold the code for clear() into the destructor:

template<typename type>
cow_ptr<type>::~cow_ptr() throw()
{
  if (count && --*count == 0)
  {
    delete count;
    delete ptr;
  }
}

This would allow clear() to delegate to the assignment operator.

template<typename type>
void cow_ptr<type>::clear()
{
  static const cow_ptr null(0, 0);
  *this = null;
}

It doesn't really matter which way you skin it; the important thing is to avoid duplication.

In one article the regular constructor looked like this:

template<typename type>
cow_ptr<type>::cow_ptr(type * p)
    : ptr(p)
    , count(new size_t(1))
{
  // all done
}

And then in the following article it looked like this:

template<typename type>
cow_ptr<type>::cow_ptr(type * p)
{
  auto_ptr<type> safe(p);
  count = new size_t(1);
  ptr = safe.release();
}

Hinges of instability are always worth close scrutiny. The problem is that new size_t(1) could throw a bad_alloc exception. If this happens should the cow_ptr destructor take responsibility for the pointer parameter or not? In the first version it doesn't, and in the second version it does. I'm not happy with either version. The trouble is that the constructor can't know whether it should take responsibility or not. The client code might require some complex initialisation. Not good. What to do? Well, why not let the client decide! One way to do this is with a dummy constructor parameter^[2].

struct bound_ptr_t;
extern const bound_ptr_t bound_ptr;

struct loose_ptr_t;
extern const loose_ptr_t loose_ptr;

template<typename type>
class cow_ptr 
{
public:
  cow_ptr(type * p, const bound_ptr_t &);
  cow_ptr(type * p, const loose_ptr_t &);
  ... ... ...
};

template<typename type>
cow_ptr<type>::cow_ptr(type * p, const bound_ptr_t &)
    : ptr(p)
    , count(new size_t(1))
{
  // all done
}

template<typename type>
cow_ptr<type>::cow_ptr(type * p, const loose_ptr_t &)
{
  auto_ptr<type> safe(p);
  count = new size_t(1);
  ptr = safe.release();
}

If you put this extra parameter into the constructor, don't forget to create a 'genuine' (no default parameters) default constructor.

template<typename type>
cow_ptr<type>::cow_ptr()
    : ptr(0)
    , count(new size_t(1))
{
  // all done
}

Notice that the default constructor could throw an exception. There is an argument that says default constructors for simple "value" classes should never throw exceptions^[3]. There are a number of ways you can solve this. Perhaps the most obvious is to not bother counting the pointer if it's null.

template<typename type>
cow_ptr<type>::cow_ptr()
    : ptr(0
    , count(0)
{
  // all done
}

This ties in nicely with the fact that count can be null anyway, thanks to clear(). However, this is not the only solution. An alternative is to not throw an exception in the first place; don't new the size_t! The insight is that we can create a static count of 1. A kind of mini singleton^[4].

template<typename type>
class cow_ptr
{
    ... ... ...
private:

  static const size_t one;
  static size_t * const shared_one;

  type * ptr;
  size_t * count;
};

template<typename type>
const size_t cow_ptr<type>::one = 1;

template<typename type>
size_t * const cow_ptr<type>::shared_one = const_cast<size_t*>(&one);

template<typename type>
cow_ptr<type>::cow_ptr()
    : ptr(p)
    , count(shared_one)
{
  // all done
}

This may seem a tad strange, but it has some unexpected benefits. The responsibility problem in the pointer parameter constructor vanishes, allowing:

template<typename type>
cow_ptr<type>::cow_ptr(type * p)
    : ptr(p)
    , count(shared_one)
{
    // all done
}

Clearly this will require some careful coding elsewhere, but it can be made to work. I'll look at this more fully next time.

Thanks to Kevlin for his comments and advice.

That's all for now. Cheers