From df379ec879c7767016e834dca56420dd63a5893f Mon Sep 17 00:00:00 2001 From: Tommaso Pecorella Date: Sun, 26 Feb 2012 19:51:19 +0100 Subject: [PATCH] Bug 1377: various memory leaks --- src/internet/model/ipv4-end-point.cc | 3 ++ src/internet/model/ipv6-end-point.cc | 3 ++ src/internet/model/tcp-socket-base.cc | 24 +++++++++-- src/internet/model/tcp-socket-base.h | 1 + src/internet/model/udp-socket-impl.cc | 40 ++++++++++++++++--- src/internet/model/udp-socket-impl.h | 1 + .../test/ipv6-dual-stack-test-suite.cc | 1 + src/network/model/socket.cc | 2 + 8 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/internet/model/ipv4-end-point.cc b/src/internet/model/ipv4-end-point.cc index 31cc177ac..d8a607ca4 100644 --- a/src/internet/model/ipv4-end-point.cc +++ b/src/internet/model/ipv4-end-point.cc @@ -40,6 +40,9 @@ Ipv4EndPoint::~Ipv4EndPoint () { m_destroyCallback (); } + m_rxCallback.Nullify (); + m_icmpCallback.Nullify (); + m_destroyCallback.Nullify (); } Ipv4Address diff --git a/src/internet/model/ipv6-end-point.cc b/src/internet/model/ipv6-end-point.cc index d76fca83b..dd2f0e506 100644 --- a/src/internet/model/ipv6-end-point.cc +++ b/src/internet/model/ipv6-end-point.cc @@ -43,6 +43,9 @@ Ipv6EndPoint::~Ipv6EndPoint () { m_destroyCallback (); } + m_rxCallback.Nullify (); + m_icmpCallback.Nullify (); + m_destroyCallback.Nullify (); } Ipv6Address Ipv6EndPoint::GetLocalAddress () diff --git a/src/internet/model/tcp-socket-base.cc b/src/internet/model/tcp-socket-base.cc index d2c3fb93c..e46828282 100644 --- a/src/internet/model/tcp-socket-base.cc +++ b/src/internet/model/tcp-socket-base.cc @@ -635,7 +635,7 @@ TcpSocketBase::SetupCallback (void) if (m_endPoint6 != 0) { m_endPoint6->SetRxCallback (MakeCallback (&TcpSocketBase::ForwardUp6, Ptr (this))); - m_endPoint6->SetDestroyCallback (MakeCallback (&TcpSocketBase::Destroy, Ptr (this))); + m_endPoint6->SetDestroyCallback (MakeCallback (&TcpSocketBase::Destroy6, Ptr (this))); } return 0; @@ -1422,8 +1422,27 @@ void TcpSocketBase::Destroy (void) { NS_LOG_FUNCTION (this); - m_node = 0; m_endPoint = 0; + if (m_tcp != 0) + { + std::vector >::iterator it + = std::find (m_tcp->m_sockets.begin (), m_tcp->m_sockets.end (), this); + if (it != m_tcp->m_sockets.end ()) + { + m_tcp->m_sockets.erase (it); + } + } + NS_LOG_LOGIC (this << " Cancelled ReTxTimeout event which was set to expire at " << + (Simulator::Now () + Simulator::GetDelayLeft (m_retxEvent)).GetSeconds ()); + CancelAllTimers (); +} + +/** Kill this socket. This is a callback function configured to m_endpoint in + SetupCallback(), invoked when the endpoint is destroyed. */ +void +TcpSocketBase::Destroy6 (void) +{ + NS_LOG_FUNCTION (this); m_endPoint6 = 0; if (m_tcp != 0) { @@ -1433,7 +1452,6 @@ TcpSocketBase::Destroy (void) { m_tcp->m_sockets.erase (it); } - m_tcp = 0; } NS_LOG_LOGIC (this << " Cancelled ReTxTimeout event which was set to expire at " << (Simulator::Now () + Simulator::GetDelayLeft (m_retxEvent)).GetSeconds ()); diff --git a/src/internet/model/tcp-socket-base.h b/src/internet/model/tcp-socket-base.h index d6af47988..ce6d4f25d 100644 --- a/src/internet/model/tcp-socket-base.h +++ b/src/internet/model/tcp-socket-base.h @@ -148,6 +148,7 @@ protected: int DoClose (void); // Close a socket by sending RST, FIN, or FIN+ACK, depend on the current state void CloseAndNotify (void); // To CLOSED state, notify upper layer, and deallocate end point void Destroy (void); // Kill this socket by zeroing its attributes + void Destroy6 (void); // Kill this socket by zeroing its attributes void DeallocateEndPoint (void); // Deallocate m_endPoint void PeerClose (Ptr, const TcpHeader&); // Received a FIN from peer, notify rx buffer void DoPeerClose (void); // FIN is in sequence, notify app and respond with a FIN diff --git a/src/internet/model/udp-socket-impl.cc b/src/internet/model/udp-socket-impl.cc index c5b58bb1d..3c11482a4 100644 --- a/src/internet/model/udp-socket-impl.cc +++ b/src/internet/model/udp-socket-impl.cc @@ -86,6 +86,11 @@ UdpSocketImpl::~UdpSocketImpl () // XXX todo: leave any multicast groups that have been joined m_node = 0; + /** + * Note: actually this function is called AFTER + * UdpSocketImpl::Destroy or UdpSocketImpl::Destroy6 + * so the code below is unnecessary in normal operations + */ if (m_endPoint != 0) { NS_ASSERT (m_udp != 0); @@ -101,6 +106,21 @@ UdpSocketImpl::~UdpSocketImpl () m_udp->DeAllocate (m_endPoint); NS_ASSERT (m_endPoint == 0); } + if (m_endPoint6 != 0) + { + NS_ASSERT (m_udp != 0); + /** + * Note that this piece of code is a bit tricky: + * when DeAllocate is called, it will call into + * Ipv4EndPointDemux::Deallocate which triggers + * a delete of the associated endPoint which triggers + * in turn a call to the method UdpSocketImpl::Destroy below + * will will zero the m_endPoint field. + */ + NS_ASSERT (m_endPoint6 != 0); + m_udp->DeAllocate (m_endPoint6); + NS_ASSERT (m_endPoint6 == 0); + } m_udp = 0; } @@ -143,27 +163,37 @@ void UdpSocketImpl::Destroy (void) { NS_LOG_FUNCTION_NOARGS (); - m_node = 0; m_endPoint = 0; - m_udp = 0; +} + +void +UdpSocketImpl::Destroy6 (void) +{ + NS_LOG_FUNCTION_NOARGS (); + m_endPoint6 = 0; } int UdpSocketImpl::FinishBind (void) { NS_LOG_FUNCTION_NOARGS (); + bool done = false; if (m_endPoint != 0) { m_endPoint->SetRxCallback (MakeCallback (&UdpSocketImpl::ForwardUp, Ptr (this))); m_endPoint->SetIcmpCallback (MakeCallback (&UdpSocketImpl::ForwardIcmp, Ptr (this))); m_endPoint->SetDestroyCallback (MakeCallback (&UdpSocketImpl::Destroy, Ptr (this))); - return 0; + done = true; } - else if (m_endPoint6 != 0) + if (m_endPoint6 != 0) { m_endPoint6->SetRxCallback (MakeCallback (&UdpSocketImpl::ForwardUp6, Ptr (this))); m_endPoint6->SetIcmpCallback (MakeCallback (&UdpSocketImpl::ForwardIcmp6, Ptr (this))); - m_endPoint6->SetDestroyCallback (MakeCallback (&UdpSocketImpl::Destroy, Ptr (this))); + m_endPoint6->SetDestroyCallback (MakeCallback (&UdpSocketImpl::Destroy6, Ptr (this))); + done = true; + } + if (done) + { return 0; } return -1; diff --git a/src/internet/model/udp-socket-impl.h b/src/internet/model/udp-socket-impl.h index 56d531f5a..86f67e741 100644 --- a/src/internet/model/udp-socket-impl.h +++ b/src/internet/model/udp-socket-impl.h @@ -108,6 +108,7 @@ private: Ptr incomingInterface); void ForwardUp6 (Ptr p, Ipv6Address saddr, Ipv6Address daddr, uint16_t port); void Destroy (void); + void Destroy6 (void); int DoSend (Ptr p); int DoSendTo (Ptr p, const Address &daddr); int DoSendTo (Ptr p, Ipv4Address daddr, uint16_t dport); diff --git a/src/internet/test/ipv6-dual-stack-test-suite.cc b/src/internet/test/ipv6-dual-stack-test-suite.cc index c5d89cc9e..133862817 100644 --- a/src/internet/test/ipv6-dual-stack-test-suite.cc +++ b/src/internet/test/ipv6-dual-stack-test-suite.cc @@ -296,6 +296,7 @@ DualStackTestCase::DoRun (void) void DualStackTestCase::DoTeardown (void) { + Simulator::Destroy (); } diff --git a/src/network/model/socket.cc b/src/network/model/socket.cc index 722a90691..66e4b2b24 100644 --- a/src/network/model/socket.cc +++ b/src/network/model/socket.cc @@ -292,6 +292,8 @@ Socket::DoDispose (void) m_connectionSucceeded = MakeNullCallback > (); m_connectionFailed = MakeNullCallback > (); + m_normalClose = MakeNullCallback > (); + m_errorClose = MakeNullCallback > (); m_connectionRequest = MakeNullCallback, const Address &> (); m_newConnectionCreated = MakeNullCallback, const Address &> (); m_dataSent = MakeNullCallback, uint32_t> ();