My previous post “Use shared_ptr inheritance rightly when design and use interfaces” has an obvious problem: There is a cyclic reference between AbstractSocketImpl and Socket I/O Streams.

typedef shared_ptr<InputStream> InputStreamPtr;
typedef shared_ptr<OutputStream> OutputStreamPtr;
typedef shared_ptr<AbstractSocketImpl> AbstractSocketImplPtr;

class AbstractSocketImpl : public AbstractSocket
{
private:
    InputStreamPtr    inputStreamPtr;
    OutputStreamPtr   outputStreamPtr;
}

class SocketInputStream : public InputStream
{
private:
    AbstractSocketImplPtr socket_impl;
}

 
When two class have shared_ptr hold each other, it will cause meory leak because of cyclic reference. When shared_ptr objects of AbstractSocketImpl and SocketInputStream are created the reference count in SocketInputStreamPtr and AbstractSocketImplPtr will count each other, so the refer_cout will be 2 of each.

auto socketImpl = make_shared<AbstractSocketImpl>(address);   //socketImpl.ref_count + 1, now socketImpl.ref_count == 1
auto inputstream = socketImpl->getInputStream();   //socketImpl.ref_count + 1 (caused by inputstream.socket_impl), now socketImpl.ref_count == 2.
//inputstream.ref_count == 2, because inputstream holds one reference and inputstream.socket_impl.inputStreamPtr holds inputstream.

 
The relationship between AbstractSocketImpl and SocketInputStream should be Composition, which SocketInputStream is initialized from an AbstractSocketImpl. There is no meaning for any operations of SocketInputStream when AbstractSocketImpl is destoried. I was incorrectly making strong reference from AbstractSocketImpl to SocketInputStream, which I was trying to manage the memory life-cycle of SocketInputStream from AbstractSocketImpl and make them high coupling. The SocketInputStream is created by AbstractSocketImpl and my mistake was trying to use a shared_ptr (strong reference) to trace the SocketInputStream object. But in fact, SocketInputStream object is managed by user not AbstractSocketImpl, AbstractSocketImpl‘s responsibility is to make sure creating only ONE SocketInputStream object, but NOT recycling it. So AbstractSocketImpl do not need to HOLD a SocketInputStream, it only need to peek whether the SocketInputStream object it created is still alive, if yes, return the pointer, if not, create the new one from itself. So I use weak_ptr to have weak reference to SocketInputStream. Following is the implementation changes:

typedef weak_ptr<InputStream> InputStreamWeakPtr;
typedef weak_ptr<OutputStream> OutputStreamWeakPtr;

class AbstractSocketImpl : public AbstractSocket
{
    InputStreamWeakPtr   wkInputStreamPtr;
    OutputStreamWeakPtr  wkOutputStreamPtr;
}

InputStreamPtr AbstractSocketImpl::getInputStream()
{
    //return make_shared<SocketInputStream>(shared_from_this());
    if ( wkInputStreamPtr.expired() )
    {
        InputStreamPtr inputstrPtr = make_shared<SocketInputStream>(shared_from_this());
        wkInputStreamPtr = inputstrPtr;
        return inputstrPtr;
    }
    return wkInputStreamPtr.lock();
}

If we want to use polymorphism in C++, normally we need to use a base interface pointer to point to an implemented class object. It is quite easy to use in normal pointer in C++. But if we use smart pointer, shared_ptr with the base class type to hold an implemented object, things going to be a little complicated.

Firstly I think it may easy going like this way:

//Point Base pointer to an implemented Derrived object
//Class "Derived" inheritances from class "Base"
typedef std::shared_ptr<Base> BasePtr;
BasePtr baseptr(new Derived());
//Then call baseptr->operations

Principle

The principle of using smart pointer to prevent memory leak is always use a named smart pointer variable to hold the result of new

But one thing need to be noted very carefully is that, you cannot use more than ONE shared_ptr to hold the same result of new:

int* ptr = new int;
shared_ptr<int> p1(ptr);
shared_ptr<int> p2(ptr); //logic error

Because each time we construct a shared_ptr object, the code will maintain 2 pointers in the object:
1. Type <T*> pointer to the object you new in the heap;
2. A “Smart Area” sp_count which holds the reference count of all the shared_ptr objects which hold the <T*>;

Each time you use copy constructor or use operate=, shared_ptr will make the reference count maintenance to plus 1 or minus 1; So if there are 2 shared_ptr objects hold the same T*, the pointer <T*> will be delete twice.

This is same when we deal with this pointer. But there is a solution: use shared_from_this():

#include <memory>
#include <iostream>

struct Good: std::enable_shared_from_this<Good>
{
    std::shared_ptr<Good> getptr() {
        return shared_from_this();
    }
};

struct Bad
{
    std::shared_ptr<Bad> getptr() {
        return std::shared_ptr<Bad>(this);
    }
    ~Bad() { std::cout << "Bad::~Bad() called\n"; }
};

int main()
{
    // Good: the two shared_ptr's share the same object
    std::shared_ptr<Good> gp1(new Good);
    std::shared_ptr<Good> gp2 = gp1->getptr();
    std::cout << "gp2.use_count() = " << gp2.use_count() << '\n';

    // Bad, each shared_ptr thinks it's the only owner of the object
    std::shared_ptr<Bad> bp1(new Bad);
    std::shared_ptr<Bad> bp2 = bp1->getptr();
    std::cout << "bp2.use_count() = " << bp2.use_count() << '\n';
} // UB: double-delete of Bad

 Interface Design

When I was trying to design an AbstracktSocket, which can return Socket I/O Streams, users can use I/O Streams smart_ptr to receive/send message through socket, just like the “Java way”:

AbstractSocketImpl implements the interface AbstractSocket, it has the getInputStream() and getOutputStream(), which will return the SocketInputStream and SocketOutputSteam. But AbstractSocketImpl holds shared_ptr of InputStream and OutputStream which implemented from AbstractSocket. SocketInputStream and SocketOutputSteam are constructed by passing AbstractSocketImpl smart_ptr into their Constructors. So when AbstractSocketImpl initialize the Socket I/O Streams, it will share this pointer. To use shared_ptr rightly, we need make AbstractSocketImpl inherit from std::enable_shared_from_this:

InputStreamPtr AbstractSocketImpl::getInputStream()
{
    if ( !inputStreamPtr )
    {
        inputStreamPtr = make_shared<SocketInputStream>(shared_from_this());
    }
    return inputStreamPtr;
}

OutputStreamPtr AbstractSocketImpl::getOutputStream()
{
    if ( !outputStreamPtr )
    {
        outputStreamPtr = make_shared<SocketOutputStream>(shared_from_this());
    }
    return outputStreamPtr;
}

You may notice that the inputStreamPtr is a shared_ptr<InputStream> type, but make_shared creates a shared_ptr<SocketInputStream> object. They are not consistent, but the compiler does not returns any error on GNU Compiler and Microsoft Windows Compiler on C++11. There is a conservative way to convert the smart pointer, by using static_pointer_cast<T> or dynamic_pointer_cast<T>:

inputStreamPtr = static_pointer_cast<InputStream>( make_shared<SocketInputStream>(shared_from_this()) );

Single-Inheritance

I have some concern about making AbstractSocketImpl inherit from std::enable_shared_from_this, why not make AbstractSocket inherit from std::enable_shared_from_this cause AbstractSocketImpl already inherits from AbstractSocket. So how to deal with the shared_from_this()? Cause the template types are different between AbstractSocket and AbstractSocketImpl.
The Solution is following:

class AbstractSocket : boost::noncopyable, public enable_shared_from_this<AbstractSocket> { ... }

class AbstractSocketImpl : public AbstractSocket
{
    std::shared_ptr<AbstractSocketImpl> shared_from_this()
    {
        return std::static_pointer_cast<AbstractSocketImpl>(AbstractSocket::shared_from_this());
    }
}

Once using enable_shared_from_this, The object must be created in Heap, NOT in Stack. Because the weak_ptr in enable_shared_from_this should be initialized. Any pointer created in Stack wrapped in shared_ptr will cause Wrong Memory Access:

// AbstractSocketImpl socketImpl(address);  //----> This is NOT right!
AbstractSocketImplPtr socketImpl = make_shared<AbstractSocketImpl>(address);
InputStreamPtr inputstream = socketImpl->getInputStream();
OutputStreamPtr outputstream = socketImpl->getOutputStream();

Multiple-Inheritance

There is a topic on Stackoverflow, wich describes the correct usage of multiple inheritance from enabled_share_from_this.