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.
<br /> typedef shared_ptr<InputStream> InputStreamPtr;<br /> typedef shared_ptr<OutputStream> OutputStreamPtr;<br /> typedef shared_ptr<AbstractSocketImpl> AbstractSocketImplPtr;</p> <p>class AbstractSocketImpl : public AbstractSocket<br /> {<br /> private:<br /> InputStreamPtr inputStreamPtr;<br /> OutputStreamPtr outputStreamPtr;<br /> }</p> <p>class SocketInputStream : public InputStream<br /> {<br /> private:<br /> AbstractSocketImplPtr socket_impl;<br /> }<br />
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.
<br /> auto socketImpl = make_shared<AbstractSocketImpl>(address); //socketImpl.ref_count + 1, now socketImpl.ref_count == 1<br /> auto inputstream = socketImpl->getInputStream(); //socketImpl.ref_count + 1 (caused by inputstream.socket_impl), now socketImpl.ref_count == 2.<br /> //inputstream.ref_count == 2, because inputstream holds one reference and inputstream.socket_impl.inputStreamPtr holds inputstream.<br />
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:
<br /> typedef weak_ptr<InputStream> InputStreamWeakPtr;<br /> typedef weak_ptr<OutputStream> OutputStreamWeakPtr;</p> <p>class AbstractSocketImpl : public AbstractSocket<br /> {<br /> InputStreamWeakPtr wkInputStreamPtr;<br /> OutputStreamWeakPtr wkOutputStreamPtr;<br /> }</p> <p>InputStreamPtr AbstractSocketImpl::getInputStream()<br /> {<br /> //return make_shared<SocketInputStream>(shared_from_this());<br /> if ( wkInputStreamPtr.expired() )<br /> {<br /> InputStreamPtr inputstrPtr = make_shared<SocketInputStream>(shared_from_this());<br /> wkInputStreamPtr = inputstrPtr;<br /> return inputstrPtr;<br /> }<br /> return wkInputStreamPtr.lock();<br /> }<br />