[Opalvoip-devel] Memory Leak in SIP endpoint

[Martin H. <mh...@tm...>]

It's maybe right not to delete if OnReceivedPDU returns true.
But OnReceivedPDU in many cases returns true not only when the pdu is handed off to a worker thread.
 
 
BOOL SIPEndPoint::OnReceivedPDU (......)
{
.........
 
PSafePtr<SIPConnection> connection = GetSIPConnectionWithLock(pdu->GetMIME().GetCallID(), PSafeReadWrite);
if (connection != NULL) {
     connection->QueuePDU(pdu);
     return TRUE;       ! ! ! ! !  In this case the worker thread handles the pdu and will delete it ! ! ! ! !
}

.........
switch (pdu->GetMethod()) {

.........

case SIP_PDU::Method_INVITE :
     return OnReceivedINVITE(transport, pdu);       ! ! ! ! ! returns false only in a case of an error ! ! ! ! !

case SIP_PDU::Method_REGISTER :
     return OnReceivedREGISTER(transport, *pdu);       ! ! ! ! ! returns false only in a case of an error ! ! ! ! !

case SIP_PDU::Method_SUBSCRIBE :
     return OnReceivedSUBSCRIBE(transport, *pdu);       ! ! ! ! ! returns false only in a case of an error ! ! ! ! !

case SIP_PDU::Method_NOTIFY :
     return OnReceivedNOTIFY(transport, *pdu);       ! ! ! ! ! returns false only in a case of an error ! ! ! ! !


........
}

 

Martin Hillmeier
 
 

  _____  

Von: Robert Jongbloed [mailto:ro...@vo...] 
Gesendet: Dienstag, 13. Mai 2008 14:32
An: Martin Hillmeier; OpalVoip-Devel List
Betreff: RE: [Opalvoip-devel] Memory Leak in SIP endpoint



Mmm, there does appear to be a leak but not where you are indicating. That function MUST NOT delete the PDU if the function returns true as that means it has been handed off to a worker thread which will delete it.

 

It can leak a PDU in the case of a NOTIFY being sent to a SUBSCRIBE handler. I have plugged that leak.

 

Robert Jongbloed

OPAL/OpenH323 Architect and Co-founder.

 

From: opa...@li... [mailto:opa...@li...] On Behalf Of Martin Hillmeier
Sent: Tuesday, 13 May 2008 5:16 PM
To: OpalVoip-Devel List (opa...@li...)
Subject: [Opalvoip-devel] Memory Leak in SIP endpoint

 

Hi all,

 

it looks like a memory leak I found in the SIP endpoint class.

In the function "SIPEndPoint::HandlePDU(OpalTransport & transport)" a new SIP_PDU object will be created.

But not in all cases it will be deleted.

 

void SIPEndPoint::HandlePDU(OpalTransport & transport)
{
  // create a SIP_PDU structure, then get it to read and process PDU
  SIP_PDU * pdu = new SIP_PDU;

 

  PTRACE(4, "SIP\tWaiting for PDU on " << transport);
  if (pdu->Read(transport)) {
    if (OnReceivedPDU(transport, pdu)) 
      return;     ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! !     function returns without deleting pdu  ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! ! !
  }
  else if (transport.good()) {
    PTRACE(2, "SIP\tMalformed request received on " << transport);
    SendResponse(SIP_PDU::Failure_BadRequest, transport, *pdu);
  }

 

  delete pdu;
}

 

 

It is right not to delete the object if the pdu is queued in the prior call to OnReceivedPDU.

But if it is not queued in OnReceivedPDU then it leaks.

In my application I have many subscriptions on other clients hence I get many NOTIFYs from the server. 

On every incoming NOTIFY it leaks memory.

 

 

Best regards 

Martin