Reference counting and API (const reference vs pointer oriented)

M

mathieu

Hi there

I have implemented a very simple smartpointer class (invasive
design). And I was wondering what should be the natural API when using
those in a Container.

I choose to define the following operator in my smartpointer class:

....
operator ObjectType * () const
{ return Pointer; }
....

which in turn forces me to have a pointer interface (instead of const
reference):

class Container {
public:
Container():Instance(0) {}
void Set(Object *o) { Instance = o; } // Pointer interface
private:
SmartPointer<Object> Instance;
};


I would have preferred a const reference interface, such as:

class Container {
public:
Container():Instance(0) {}
void Set(Object const &o) { Instance = const_cast<Object*>(&o); }
private:
SmartPointer<Object> Instance;
};

Should I refactor my smartpointer class to be more reference oriented
instead of pointer oriented. I do not see a case where the smart
pointer should point to a NULL pointer.

Comments ?

thanks
-Mathieu


// Full code:
#include <assert.h>

template<class ObjectType>
class SmartPointer
{
public:
SmartPointer():pointer(0) {}
SmartPointer(const SmartPointer<ObjectType>& p):pointer(p.Pointer)
{ Register(); }
SmartPointer(ObjectType* p):pointer(p)
{ Register(); }
~SmartPointer() {
UnRegister();
Pointer = 0;
}
ObjectType *operator -> () const
{ return Pointer; }
operator ObjectType * () const
{ return Pointer; }
void operator = (SmartPointer const &r)
{ return operator = (r.GetPointer()); }
void operator = (ObjectType *r)
{
if(Pointer) Pointer->UnRegister();
Pointer = r;
if(Pointer) Pointer->Register();
}
private:
void Register() { if(Pointer) Pointer->Register(); }
void UnRegister() { if(Pointer) Pointer->UnRegister(); }

ObjectType* Pointer;
};

class Object
{
public:
Object():ReferenceCount(0) {}
virtual ~Object() { assert(ReferenceCount == 0); }

void Register() {
ReferenceCount++;
}
void UnRegister() {
ReferenceCount--;
if(!ReferenceCount) delete this;
}
private:
long ReferenceCount;
};

class Foo : public Object
{
public:
int F;
};

class Container {
public:
Container():Instance(0) {}
void Set(Object *o) { Instance = o; }
private:
SmartPointer<Object> Instance;
};

int main()
{
SmartPointer<Foo> o = new Foo;
Container c;
c.Set( o );
return 0;
}
 
J

Juha Nieminen

mathieu said:
class Object
{
public:
Object():ReferenceCount(0) {}
virtual ~Object() { assert(ReferenceCount == 0); }

void Register() {
ReferenceCount++;
}
void UnRegister() {
ReferenceCount--;
if(!ReferenceCount) delete this;
}
private:
long ReferenceCount;
};

class Foo : public Object
{
public:
int F;
};

You have a basic problem with your reference-counting base class there.

Assume you have one instance of Foo allocated behind one SmartPointer
(let's name it fooPtr1). The reference count of that Foo instance will be 1.
Also assume you have another instance of Foo allocated and shared
between two SmartPointers (let's name them fooPtr2 and fooPtr3). The
reference count of this instance will be 2.

Now assume you do a "*fooPtr1 = *fooPtr2;", which is not in any way a
far-fetched thing to do. What happens?

What happens is that now the object behind fooPtr1 will have its
reference counter set to 2, even though only one smart pointer is
pointing to it. When fooPtr1 is destroyed, it will decrement that
counter to 1, not destroy the object, and you have a memory leak.

Assume the inverse situation: "*fooPtr2 = *fooPtr1;" What happens?

What happens is that now the second object will have its reference
counter set to 1 even though two smart pointers are pointing to it. When
one of then is destroyed, the other will point to freed memory. Kaboom.

The solution to this problem is rather simple, though.
 
M

mathieu

  You have a basic problem with your reference-counting base class there.

  Assume you have one instance of Foo allocated behind one SmartPointer
(let's name it fooPtr1). The reference count of that Foo instance will be 1.
  Also assume you have another instance of Foo allocated and shared
between two SmartPointers (let's name them fooPtr2 and fooPtr3). The
reference count of this instance will be 2.

  Now assume you do a "*fooPtr1 = *fooPtr2;", which is not in any way a
far-fetched thing to do. What happens?

  What happens is that now the object behind fooPtr1 will have its
reference counter set to 2, even though only one smart pointer is
pointing to it. When fooPtr1 is destroyed, it will decrement that
counter to 1, not destroy the object, and you have a memory leak.

  Assume the inverse situation: "*fooPtr2 = *fooPtr1;" What happens?

  What happens is that now the second object will have its reference
counter set to 1 even though two smart pointers are pointing to it. When
one of then is destroyed, the other will point to freed memory. Kaboom.

  The solution to this problem is rather simple, though.

Move the cstor/dstor of Object to the protected: section ?
I realized that only now, but that does not work for derived class.

Any other suggestion I might be missing ?

Thanks
-Mathieu
 
J

Juha Nieminen

mathieu said:
Any other suggestion I might be missing ?

Yes: Implement a copy constructor and assignment operator in your
reference-counting base class which do *not* copy the reference counter
member variable. (If there's nothing else in the base class, then simply
make empty implementations of them.)

This way you can freely copy/assign derived objects without messing up
the reference counters.
 
M

mathieu

Yes: Implement a copy constructor and assignment operator in your
reference-counting base class which do *not* copy the reference counter
member variable. (If there's nothing else in the base class, then simply
make empty implementations of them.)

This way you can freely copy/assign derived objects without messing up
the reference counters.

It indeed solve the issue you reported, but does not solve the issue
where one would do:

Foo f = *fooPtr3;

Thanks again
-Mathieu
 
J

Juha Nieminen

mathieu said:
It indeed solve the issue you reported, but does not solve the issue
where one would do:

Foo f = *fooPtr3;

I don't see what's wrong with that.
 
M

mathieu

  I don't see what's wrong with that.

<quote>
Yes: Implement a copy constructor and assignment operator in your
reference-counting base class which do *not* copy the reference
counter
member variable. (If there's nothing else in the base class, then
simply
make empty implementations of them.)
</quote>

Which lead to:

Object(const Object&){ } // see how ReferenceCount is *never*
initialized
void operator=(const Object&){ }

Thus when you do:

Foo f = *fooPtr3;
// f.ReferenceCount is pretty much random memory

thanks
-Mathieu
 
M

mathieu

<quote>
Yes: Implement a copy constructor and assignment operator in your
reference-counting base class which do *not* copy the reference
counter
member variable. (If there's nothing else in the base class, then
simply
make empty implementations of them.)
</quote>

Which lead to:

  Object(const Object&){  } // see how ReferenceCount is *never*
initialized
  void operator=(const Object&){  }

Thus when you do:

  Foo f = *fooPtr3;
  //  f.ReferenceCount is pretty much random memory

thanks
-Mathieu

Alright found the answer in:

http://www.parashift.com/c++-faq-lite/freestore-mgmt.html#faq-16.24

And copy cstor need to initialize the ref count:

Object(const Object&):ReferenceCount(0){}
void operator=(const Object&){ }

Thanks the ref counting is much user friendly !

-Mathieu
 
J

Juha Nieminen

mathieu said:
<quote>
Yes: Implement a copy constructor and assignment operator in your
reference-counting base class which do *not* copy the reference
counter
member variable.

I said you should not copy the reference counter. I didn't say you
shouldn't initialize it.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,734
Messages
2,569,441
Members
44,832
Latest member
GlennSmall

Latest Threads

Top