5 Aug
2010
5 Aug
'10
9:32 a.m.
Hi Francisco,
thanks for the bug report! I've put the fix into the CVS.
Regards, Jan
--
Jan Willamowius, jan@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.
>