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