Menu

#514 Assert in SIPTransaction::OnRetry()

Development_Branch
closed-fixed
nobody
None
5
2014-05-28
2014-05-26
No

We can't propertly use m_transport in SIPTransaction::OnRetry() because at the same time m_transport may be changed.

In my case when OnRetry() is executing SIP_PDU_Work::Work() calls SIP_PDU::SetTransport with new transport.

Discussion

  • Sysolyatin Pavel

    This is log:

    575788  14-05-26 12:27:50,154 (MCUTest.MassCalls) DEBUG           SIP Pool:5408         sipep.cxx(2216)  SIP        Retransmitting previous response for transaction id=z9hG4bK6722966c-dd0c-1910-8be0-94de800be7dd
    575789  14-05-26 12:27:50,155 (MCUTest.MassCalls) INFO           SIP Pool:6968        sippdu.cxx(3390)  SIP        SIPTransaction::OnRetry(), 0B255F38, &m_transport == 0B2561B8, m_transport == 0891E610
    575790  14-05-26 12:27:50,155 (MCUTest.MassCalls) DEBUG           SIP Pool:6968    transports.cxx(1454)  OpalUDP        Setting interface to 192.168.201.30%Qualcomm Atheros AR8161/8165 PCI-E Gigabit Ethernet Controller (NDIS 6.20)
    575791  14-05-26 12:27:50,155 (MCUTest.MassCalls) INFO           SIP Pool:5408        sippdu.cxx(1911)  SIP        SIP_PDU::SetTransport(): this == 0B255F38, &m_transport == 0B2561B8, m_transport == 0891E610, new transpoort == 0B3509A0
    575792  14-05-26 12:27:50,156 (MCUTest.MassCalls) DEBUG           SIP Pool:5408    transports.cxx(1393)  Opal        Destroy transport == 0891E610
    575793  14-05-26 12:27:50,156 (MCUTest.MassCalls) DEBUG           SIP Pool:5408    transports.cxx(979)  Opal        Transport Close
    
     
  • Robert Jongbloed

    I have checked in a possible fix for this.

     
  • Robert Jongbloed

    • status: open --> closed-fixed
     
  • Valeriy V. Argunov

    Now OPAL crashes while calling SIP_PDU::~SIP_PDU() -> SIP_PDU::SetTransport(NULL).
    I see that safeReferenceCount is 0 so it seems like the object was destroyed already.
    When I remove last ReadWriteLock/ReadWriteUnlock changes, then everything works as before.

    Stack trace:

    [Frames below may be incorrect and/or missing]  
    ptlibd.dll!PWaitAndSignal::PWaitAndSignal(const PSync & sem={...}, bool wait=true) Line 123 C++
    ptlibd.dll!PReadWriteMutex::StartNest() Line 2692   C++
    ptlibd.dll!PReadWriteMutex::StartWrite() Line 2817  C++
    ptlibd.dll!PSafeObject::LockReadWrite() Line 143    C++
    

    opald.dll!SIP_PDU::SetTransport(const PSafePtr<OpalTransport,PSafePtrBase> & transport={...}) Line 1909 C++
    opald.dll!SIP_PDU::~SIP_PDU() Line 1903 C++
    opald.dll!SIP_PDU::`scalar deleting destructor'(unsigned int) C++
    opald.dll!std::allocator<SIP_PDU>::destroy<SIP_PDU>(SIP_PDU * _Ptr=0x0557b058) Line 623 C++
    opald.dll!std::allocator_traits<std::allocator<SIP_PDU> >::destroy<SIP_PDU>(std::allocator<SIP_PDU> & _Al={...}, SIP_PDU * _Ptr=0x0557b058) Line 758 C++
    opald.dll!std::_Wrap_alloc<std::allocator<SIP_PDU> >::destroy<SIP_PDU>(SIP_PDU * _Ptr=0x0557b058) Line 909 C++
    opald.dll!std::deque<SIP_PDU,std::allocator<SIP_PDU> >::pop_front() Line 1464 C++
    opald.dll!std::queue<SIP_PDU,std::deque<SIP_PDU,std::allocator<SIP_PDU> > >::pop() Line 161 C++
    opald.dll!SIPConnection::OnReceivedACK(SIP_PDU & ack={...}) Line 3122 C++
    opald.dll!SIPConnection::OnReceivedPDU(SIP_PDU & pdu={...}) Line 2175 C++
    opald.dll!SIP_PDU_Work::Work() Line 2233 C++

    Also.. Is it really needed to make

    if (m_transport != NULL) {
    PTRACE(5, "SIP\tDereferenced transport 0x" << m_transport << " from 0x" << this << ' ' << *this);
    m_transport->Dereference();
    }

    and then

    if (m_transport != NULL) {
    PTRACE(5, "SIP\tReferenced transport 0x" << m_transport << " from 0x" << this << ' ' << *this);
    m_transport->Reference();
    }

    ? Code

    const_cast<OpalTransportPtr &="">(m_transport) = transport;

    references/dereferences transports already.

     
  • Robert Jongbloed

    It is a different reference count to the PSafePtr one. The code is as it should be.

    Check in another change so does not try and lock the object as it is deleted, though that should still work, and certainly does do so here.

     
  • Valeriy V. Argunov

    About the reference count.. OK, I missed this.

     
  • Valeriy V. Argunov

    I can confirm that OPAL doesn't crash now.

     

Log in to post a comment.