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();
}