From e18b54733abb9bfc493f37a71dd452f62b4f3b18 Mon Sep 17 00:00:00 2001 From: Ewgenij Starostin Date: Mon, 29 Jun 2009 14:35:32 -0700 Subject: [PATCH] Fix 559, cpSocketImpl doesnt free endpoint quickly enough after being closed --- src/internet-stack/tcp-socket-impl.cc | 37 ++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/internet-stack/tcp-socket-impl.cc b/src/internet-stack/tcp-socket-impl.cc index 1f1f21bbc..82edb9762 100644 --- a/src/internet-stack/tcp-socket-impl.cc +++ b/src/internet-stack/tcp-socket-impl.cc @@ -172,13 +172,18 @@ TcpSocketImpl::~TcpSocketImpl () if (m_endPoint != 0) { NS_ASSERT (m_tcp != 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 ::Destroy below - * will will zero the m_endPoint field. + /* + * Note that this piece of code is seriously convoluted: When we do a + * Bind we allocate an Ipv4Endpoint. Immediately thereafter we always do + * a FinishBind which sets the DestroyCallback of that endpoint to be + * TcpSocketImpl::Destroy, below. When m_tcp->DeAllocate is called, it + * will in turn call into Ipv4EndpointDemux::DeAllocate with the endpoint + * (m_endPoint). The demux will look up the endpoint and destroy it (the + * corollary is that we don't own the object pointed to by m_endpoint, we + * just borrowed it). The destructor for the endpoint will call the + * DestroyCallback which will then invoke TcpSocketImpl::Destroy below. + * Destroy will zero m_node, m_tcp and m_endpoint. Yes, we zeroed m_node + * above and will zero m_tcp below as well; but that's what it does. */ NS_ASSERT (m_endPoint != 0); m_tcp->DeAllocate (m_endPoint); @@ -694,6 +699,24 @@ Actions_t TcpSocketImpl::ProcessEvent (Events_t e) << m_state << " event " << e << " set CloseNotif "); } + + if (m_state == CLOSED && saveState != CLOSED && m_endPoint != 0) + { + NS_ASSERT (m_tcp != 0); + /* + * We want to deallocate the endpoint now. We can't just naively call + * Deallocate (see the comment in TcpSocketImpl::~TcpSocketImpl), we + * have to turn off the DestroyCallback to keep it from calling back + * into TcpSocketImpl::Destroy and closing pretty much everything down. + * Once we have the callback disconnected, we can DeAllocate the + * endpoint which actually deals with destroying the actual endpoint, + * and then zero our member varible on general principles. + */ + m_endPoint->SetDestroyCallback(MakeNullCallback()); + m_tcp->DeAllocate (m_endPoint); + m_endPoint = 0; + } + return stateAction.action; }