I get an access exception on Release() on GCC but worked on different
compiler, I am trying to figure out if there is anything with this code
in a header:
class CountObject {
private:
long count;
public:
CountObject() : count(0) {}
CountObject(const CountObject&) : count(0) {}
virtual ~CountObject() {}
CountObject& operator=(const CountObject &) { return *this; }
void Grab() { count++; }
void Release() {
if (count > 0) count--; //< Exception Here
if (count <= 0) {
delete this;
}
}
// attribute functions
long Count() const { return count; }
};
Any insight would be helpful. What is the best way (most standard) to
prevent this problem.
...
Well, if you do something like this
CountObject* p = new CountObject;
p->Grab();
p->Release();
p->Release();
(i.e. call 'Release' more times than you call 'Grab') the second call to
'Release' will lead to undefined behavior, since the object has already been
destroyed, which in practice will often crash at the first attempt to access any
data field inside 'Release'. This could be exactly what happens in your case.
A couple of words on the design of your code. The implementation of your
'Release' method begins with 'if (count > 0)', which means that you expect that
'Release' can be called with 'count' equal to zero. This, in turn, means that
you actually expect that there might be more calls to 'Release' in your program
than calls to 'Grab'. This is, in my opinion, a rather strange thing to allow.
The way it is implemented now, the only safe scenario when you can call
'Release' with 'count' equal to zero is when the object never "grabbed" but
still "released", as in
CountObject* p = new CountObject;
...; // No 'Grab's
p->Release(); // OK, kills the object
There's no other scenario that can legally lead to the 'count == 0' situation in
'Release'. You probably have to ask yourself: do you really have to allow the
'count == 0' situation in 'Release'? I'd suggest that you better make it
illegal, i.e. require that there's an equal number of 'Grab' and 'Release' calls
in the program. In that case the 'Release' method might look as follows:
void Release() {
assert(count > 0);
if (--count == 0)
delete this;
}
This will outlaw the above 'no-Grab' variant and the user will always have to do
an explicit 'Grab' every time he creates a reference to the object:
CountObject* p = new CountObject;
p->Grab();
...
p->Release();
To make thing easier for the user it might make sense to call 'Grab' internally
from the object's constructor (i.e. essentially start with 'count' set to 1, not
0). In that case, there's no need to 'Grab' the object immediately after its
creation
CountObject* p = new CountObject;
...; // No 'Grab's
p->Release(); // OK, kills the object
and there's a need to do a 'Grab' only when extra references to the object are
introduced:
CountObject* p = new CountObject;
// No 'Grab' needed
...
CountObject* q = p;
q->Grab(); // Extra reference - do a 'Grab'
...
p->Release();
...
q->Release();
Choose which approach you like better.
Of course, there's also an option to go with a third-party reference-counting
approach instead of inventing your own (see Boost library, for example).
