Bug 664 - memory fault/dangling pointer problems in tcp-socket-impl.cc, with suggested fixes
memory fault/dangling pointer problems in tcp-socket-impl.cc, with suggested ...
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: internet
ns-3-dev
All All
: P1 blocker
Assigned To: ns-bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-21 09:31 EDT by Bill Roome
Modified: 2009-10-07 23:13 EDT (History)
2 users (show)

See Also:


Attachments
Revised version of source. Works for me. However, it's based on 3.5 stable, not dev. (56.69 KB, patch)
2009-08-21 09:31 EDT, Bill Roome
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bill Roome 2009-08-21 09:31:49 EDT
Created attachment 566 [details]
Revised version of source. Works for me. However, it's based on 3.5 stable, not dev.

I found the following bugs in stable version 3.5 of internet-stack/tcp-socket-
impl.cc. Symptoms are crashing with a memory fault and/or getting stuck in a loop when the simulation ends.

1. TcpSocketImpl::PersistTimeout() uses m_pendingData without verifying that it's not 0. It's clear that the lines up to and including SendPacket() should be guarded with a test, as in:

     if (!m_pendingData) {
         Ptr<p> = m_pendingData->...; ...;
         m_tcp->SendPacket (...);
     }

But I'm not sure about last few lines, where it reschedules the event.
Should those be invoked if m_pendingData is 0?

2. Sometimes the socket is deleted when there is a pending m_rextEvent. As a result, the scheduler calls the handler function on the freed space, with the usual nasty result.

I think the solution is to cancel the event in TcpSocketImpl::ProcessEvent(), when it detects that the socket is closed:

    if (m_state == CLOSED && saveState != CLOSED && m_endPoint != 0) {
       .....
       m_rextEvent.Cancel ();  // ADD
    }

3. Under heavy load, the m_persistEvent event isn't always canceled before the socket is deleted. I don't know where it's supposed to be canceled, but one solution is to cancel it in the d'tor. And for safety, it would be nice to assert that the other events are no longer active.

Eg, add the following to TcpSocketImpl::~TcpSocketImpl():

  NS_ASSERT (m_retxEvent.IsExpired ());
  NS_ASSERT (m_lastAckEvent.IsExpired ());
  NS_ASSERT (m_delAckEvent.IsExpired ());
  m_persistEvent.Cancel ();

1 & 3 are also a problem in version 3.4.

I've attached a modified version that works for me. It's based on stable version 3.5.
Comment 1 Josh Pelkey 2009-10-07 23:13:53 EDT
changeset c54b36fb7317