[h323plus] Memory leak in gkserver.cxx

Jan Willamowius jan at willamowius.de
Thu Aug 5 09:32:38 EDT 2010


Hi Francisco,

thanks for the bug report!
I've put the fix into the CVS.

Regards,
Jan

-- 
Jan Willamowius, jan at willamowius.de, http://www.gnugk.org/


Francisco Olarte (M) wrote:
> We've been working with and old version of openh233 which had a memory
> leak in H323GatekeeperServer::OnAdmission ( it was tested, we had call
> counters and noticed a call was leaked whenever we rejected it ).
> 
> We are now trying to switch to h323plus, and I've noticed the memory
> leak still is in place, so I've subscribed to report it.
> 
> The method in question is H323GatekeeperServer::OnAdmission. I've
> checked the issue in the last CVS version and it still persists.
> 
> Below is a copy of the method with the ( working in many millions
> calls across last months ) fix we are using in our code in the middle.
> 
> If it's neccessary I can try to make a patch, but I prefer to give
> this in context.
> 
> 
> 
> >>>>>>>>>>>>>>>>>
> H323GatekeeperRequest::Response
> H323GatekeeperServer::OnAdmission(H323GatekeeperARQ & info)
> {
>   PTRACE_BLOCK("H323GatekeeperServer::OnAdmission");
> 
>   OpalGloballyUniqueID id = info.arq.m_callIdentifier.m_guid;
>   if (id == NULL) {
>     PTRACE(2, "RAS\tNo call identifier provided in ARQ!");
>     info.SetRejectReason(H225_AdmissionRejectReason::e_undefinedReason);
>     return H323GatekeeperRequest::Reject;
>   }
> 
>   H323GatekeeperRequest::Response response;
> 
>   PSafePtr<H323GatekeeperCall> oldCall = FindCall(id, info.arq.m_answerCall);
>   if (oldCall != NULL)
>     response = oldCall->OnAdmission(info);
>   else {
>     // If on restarted in thread, then don't create new call, should already
>     // have had one created on the last pass through.
>     if (!info.IsFastResponseRequired() && info.CanSendRIP()) {
>       PTRACE(2, "RAS\tCall object disappeared after starting slow PDU
> handler thread!");
>       info.SetRejectReason(H225_AdmissionRejectReason::e_undefinedReason);
>       return H323GatekeeperRequest::Reject;
>     }
> 
> ************************************************************
> *****  Here newcall is allocated in a plain ptr.
> ************************************************************
> 
>     H323GatekeeperCall * newCall = CreateCall(id,
>                             info.arq.m_answerCall ?
> H323GatekeeperCall::AnsweringCall
>                                                   :
> H323GatekeeperCall::OriginatingCall);
>     PTRACE(3, "RAS\tCall created: " << *newCall);
> 
>     response = newCall->OnAdmission(info);
> 
> 
> ************************************************************
> ******* This tracks the new call inside the endpoint WHEN NOT REJECTED
> ************************************************************
> 
>     if (response != H323GatekeeperRequest::Reject) {
>       mutex.Wait();
> 
>       info.endpoint->AddCall(newCall);
>       oldCall = activeCalls.Append(newCall);
> 
>       if (activeCalls.GetSize() > peakCalls)
>         peakCalls = activeCalls.GetSize();
>       totalCalls++;
> 
>       PTRACE(2, "RAS\tAdded new call (total=" << activeCalls.GetSize()
> << ") " << *newCall);
>       mutex.Signal();
> 
>       AddCall(oldCall);
>     }
>   }
> ************************************************************
> ******* But call is leaked when rejected, we fixed it using:
> ************************************************************
>     else {
>       PTRACE(2,"RAS\tDeleting rejected call: "<<*newCall);
>       delete newCall;
>     }
> 
> ************************************************************
> *****
> ***** I've traced code path to my best and think newcall cannot
> ***** be deleted by onAdmission, and our code has not got any
> ***** double-delete, but we discovered this in final testing
> ***** and using a H323GatekeeperCall derived call class (
> ***** which counts instances, that is how we discovered it ).
> *****
> ****** End of comments/modifications.
> ************************************************************
> 
> #ifdef H323_H248
>   switch (response) {
>     case H323GatekeeperRequest::Confirm :
>       if (oldCall->AddCallCreditServiceControl(info.acf.m_serviceControl))
>         info.acf.IncludeOptionalField(H225_AdmissionConfirm::e_serviceControl);
>       break;
> 
>     case H323GatekeeperRequest::Reject :
>       if (oldCall != NULL &&
> oldCall->AddCallCreditServiceControl(info.arj.m_serviceControl))
>         info.arj.IncludeOptionalField(H225_AdmissionReject::e_serviceControl);
>       break;
> 
>     default :
>       break;
>   }
> #endif
> 
>   return response;
> }
> 
> <<<<<<<<<<<<<<<<<<<<<
> 
> 
> Regards.
>     Francisco Olarte.
> 



More information about the h323plus mailing list