issue in smart pointer class - C++ Unleashed Book - why is destructor called

[yTkN8vqX]

I was testing the smart pointer code from Jesse Liberty's book C++ Unleashed and noticed in testing it did not work, so I am trying to fix it. The problem was in the assignment to non const char* on [] operator both the new and old instances where decremented to zero and thus releasing the memory causing invalid pointer access in the rest of the application. I commented out the override of the = operator to stop one of them from being set to zero and released for now, however the other one has me stumped.

it appears on the line
rcString = new countedString(rcString->cString);
in the method:
char & String::operator[](int index)

is calling a destructor after the two constructors are called
and thus causing the release of the other memory instance
(i use codeblocks ide)

My question is why is this destructor called?

The code is below

#include <iostream>
#include <string>
using namespace std;

template<class T>
class RCSmartPointer
{
public:
RCSmartPointer(T* ptrToRC = 0);
RCSmartPointer(const RCSmartPointer& rhs);
~RCSmartPointer();

//RCSmartPointer& operator=(const RCSmartPointer& rhs);

T* operator->() const;
T& operator*() const;

private:
T * pRC;
void initialize();
};

template<class T>
void RCSmartPointer<T>::initialize()
{
        cout << " 00025" << endl;
        cout << " initialize() : stop AA" << endl;
        if ( pRC == 0 ) return;
        cout << " initialize() : stop BB" << endl;
        if ( pRC->isShareable() == false )
        {
            cout << " initialize() : stop CC" << endl;
            pRC = new T(*pRC);
            cout << " initialize() : stop DD" << endl;
        }
        pRC->addRef();
        cout << " initialize() : stop EE" << endl;
}

template<class T>
RCSmartPointer<T>::RCSmartPointer(T* ptrToRC):
pRC(ptrToRC)
{
        cout << " 00026" << endl;
        cout << " hhhh" << endl;
        initialize();
}

template<class T>
RCSmartPointer<T>::RCSmartPointer(const RCSmartPointer& rhs):
pRC(rhs.pRC)
{
        cout << " 00006" << endl;
        initialize();
}

template<class T>
RCSmartPointer<T>::~RCSmartPointer()
{
        cout << " 00007" << endl;
        cout << "     .....pRC{" ;    pRC->ShowAddress();
        cout << "} = " << endl;
        if ( pRC )
        {
            cout << " 00008" << endl;
            pRC->removeRef();
        }
}
/*
template<class T>
RCSmartPointer<T>& RCSmartPointer<T>::operator=(const RCSmartPointer& rhs)
{
        cout << " 00027 " << " equals operator" << endl;
        cout << "     .....pRC{" ;    pRC->ShowAddress();
        cout << "} = " << endl;
        cout << "     .....rhs.pRC{" ;    rhs.pRC->ShowAddress();
        cout << "} = " << endl;
        if ( pRC == rhs.pRC )
        return *this;

        if ( pRC )
        {
           cout << " 00004" << "{" ;
           pRC->ShowAddress();
           cout << "}" << endl;
           pRC->removeRef();
        }
        cout << " 00005" << endl;
        pRC = rhs.pRC;
        cout << " 00005a" << endl;
        return *this;
}
*/
template<class T>
T* RCSmartPointer<T>::operator->() const
{
        //cout << " 00035" << endl;
        return pRC;
}

template<class T>
T& RCSmartPointer<T>::operator*() const
{
        //cout << " 00036" << endl;
        return *pRC;
}


class ReferenceCounted
{
public:
void addRef() { ++referenceCount; cout << " ..addRef : referenceCount = " << referenceCount << endl; }
void removeRef() {
    cout << " ..removeRef : referenceCount[before] = " << referenceCount << endl;
if ( --referenceCount == 0 ) { delete this; cout << " called delete this" << endl; }
cout << " ..removeRef : referenceCount[after] = " << referenceCount << endl;
}

void markNotShareable() { shareable = false; }
bool isShareable() { return shareable; }
bool isShared() { return referenceCount > 1; }

protected:
ReferenceCounted():referenceCount(0), shareable(true) {}
ReferenceCounted(const ReferenceCounted& rhs):referenceCount(0), shareable(true) {}
ReferenceCounted& operator=(const ReferenceCounted& rhs) { return *this; }
virtual ~ReferenceCounted(){}

private:
int referenceCount;
bool shareable;
};





class String
{
public:

String(const char * cString = ""):rcString(new countedString(cString)){}

char operator[](int index) const { cout << " 00023" << endl;   return rcString->cString[index]; }
char& operator[](int index);
operator char*() const { cout << " 00024" << endl;   return rcString->cString; }

void ShowCountedStringAddress() const { rcString->ShowAddress(); }

private:
struct countedString : public ReferenceCounted
{
char * cString;

countedString( const char * initialCString) { cout << " mmm" << endl; initialize(initialCString); cout << " nnn\n"; }
countedString( const countedString & rhs) { cout << " jjj" << endl; initialize(rhs.cString); }
~countedString()
{
        delete [] cString;
}
void initialize(const char * initialCString)
{
        cout << " 00022" << endl;
        cout << " llll1" << endl;
        cString = new char[strlen(initialCString) +1];
        cout << " llll2" << endl;
        strcpy(cString,initialCString);
        cout << " llll3" << endl;
}

void ShowAddress() const { cout << &cString; }

friend class RCSmartPointer<countedString>;
};

RCSmartPointer<countedString> rcString;
};

char & String::operator[](int index)
{
        cout << " 00021 : operator[] : rcString = " ;
        rcString->ShowAddress() ;
        cout << endl;
        if ( rcString->isShared())
        {
            rcString->removeRef();
            cout << " operator[] : marker A" << endl;

            rcString = new countedString(rcString->cString); // why destructor called here
            destructor called before this?
            rcString->addRef();
            cout << " operator[] : marker b" << endl;
        }
        rcString->markNotShareable();
        cout << " operator[] : marker c" << endl;
        return rcString->cString[index];
}


int main()
{
        cout << " .... marker 1" << endl;
        String s1("test1");
        cout << " .... marker 2" << endl;
        String s2 = s1;
        cout << " .... marker 3" << endl;
        //String s2("testy");
        cout << endl << " showAddress[1]=";
        s1.ShowCountedStringAddress();
        cout << endl << " showAddress[1A]=";
        s2.ShowCountedStringAddress();
        cout << " .... marker 4" << endl;
        cout << endl;
        cout << " s1: " << s1 << " " << endl;
        cout << " s2: " << s2 << " " << endl;
        cout << " .... marker 5" << endl;
        char& ch1 = s1[2]; // this is where it all goes wrong
        cout << " .... marker 6" << endl;
        cout << " ch1 = " << ch1 << endl;
        cout << " s1: " << s1 << " " << endl;
        cout << " s2: " << s2 << " " << endl;
        cout << " .... marker 7" << endl;
        ch1 = 'b';
        cout << " s1: " << s1 << " " << endl;
        cout << " ch1 = " << ch1 << endl;
        cout << endl << " showAddress[2]=";
        s1.ShowCountedStringAddress();
        cout << endl;
        return 0;
}

[kbw]

char& String::operator[](int index) is a bit of a mess.

Reference counting works in two parts. First, there's the actual object that you want to reference count. That's wrapped in a class that has the count and provides methods for checking and changing the count. Secondly, there's the wrapper that clients use. These are created/copied/deleted with abandon, but underneath, it changes count of the wrapped object. When the reference count drops to zero, it releases the wrapped object. So the two work in tandem.

Your problem is two fold. First, your trace code is calling "reference" after it has been deleted in ~RCSmartPointer(). But more importantly, char& String::operator[](int index) is bad.

char& String::operator[](int index) returns a reference to the string. So the underlying string has to be locked down before a reference to it can be given out. The lock down process removes the "shared" flag. And if it was shared, it creates a new instance.

Rather than creating the wrapped object, the code should create a wrapper object. Because it's creating the wrapped, the reference count isn't incremented on creation, so it's deleted in the assignment before can be used, then the actual use attempts to use a deleted object.

The corrected code is:

template<class T>
RCSmartPointer<T>& RCSmartPointer<T>::operator=(const RCSmartPointer& rhs)
{
	if (pRC == rhs.pRC)
		return *this;

	if (pRC)
	{
		pRC->removeRef();
	}

	rhs.pRC->addRef();
	pRC = rhs.pRC;
	return *this;
}

char& String::operator[](int index)
{
	if (rcString->isShared())
	{
		rcString = RCSmartPointer<countedString>(new countedString(rcString->cString));
		rcString->markNotShareable();
	}
	return rcString->cString[index];
}

[yTkN8vqX]

thank you for your help, it is much appreciated

it has been years since I programmed C++ last and I am trying to get up to speed again