[Home]

Summary:ASTERISK-25299: RTP port leaks with incoming OOH323 calls
Reporter:Alexandr Dranchuk (dav)Labels:
Date Opened:2015-08-03 12:58:29Date Closed:2017-11-03 19:02:33
Priority:MajorRegression?
Status:Closed/CompleteComponents:Addons/chan_ooh323
Versions:11.19.0 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Debian 8.1 x86_64 Attachments:( 0) full.log
( 1) h323_log.txt
( 2) rtpleak.diff
Description:When we get incoming call thru OOH323 channel with "early media" enabled (MediaWaitForConnect=no;  faststart=yes; tunneling=yes)
and during ringing call is canceled by caller, the RTP ports are not closed causing RTP port leaking.
Comments:By: Asterisk Team (asteriskteam) 2015-08-03 12:58:31.253-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].
By: Alexander Anikin (may213) 2015-08-03 16:08:08.026-0500

Hi,

I'm not sure that it's ooh323 problem but will test this issue.

By: Alexander Anikin (may213) 2015-08-03 16:56:51.265-0500

Alexandr,

I've checked asterisk on two heavy loaded on h.323 asterisk servers.
One have some 11-svn version and second have 13.2.0

Both of them have processed more than 1000000 calls and haven't such leak.

Looks like to this problem exist on asterisk version that you tested only.
Could you please provide more logs for the issue?


By: Alexandr Dranchuk (dav) 2015-08-03 23:04:44.779-0500

Hi Alexandr, just compared source code for 13.4.0 and 11.18.0 that I'm using, the BUG in 11.18.0 are NOT fixed in 13.4.0
I found it: (2 weeks of learning and debuging ooh323)
it's when we get call state OO_CALL_CLEARED but Logical Channels still open, and according the code they never closed since function not reached in ooEndCall()
and ooCleanCall() not called.

here is the solution (logs confirming the issue will attach):

for now:
{code:title=ooCalls.c|borderStyle=solid}
int ooEndCall(OOH323CallData *call)
{
  OOTRACEDBGA4("In ooEndCall call state is - %s (%s, %s)\n",
                ooGetCallStateText(call->callState), call->callType,
                call->callToken);

  if(call->callState == OO_CALL_REMOVED) {
     OOTRACEINFO2("Call already removed %s\n",
                   call->callToken);
    return OO_OK;
  }

  if (call->callIdentifier.guid.numocts == 0) call->callState = OO_CALL_CLEARED;

  if(!call->pH225Channel || call->pH225Channel->sock ==0)
  {
     call->callState = OO_CALL_CLEARED;
  }

  if(call->callState == OO_CALL_CLEARED || call->callState == OO_CALL_CLEAR_RELEASESENT)
  {
     ooCleanCall(call);
     call->callState = OO_CALL_REMOVED;
     return OO_OK;
  }
*strong*
  if(call->logicalChans)
  {
     OOTRACEINFO3("Clearing all logical channels. (%s, %s)\n", call->callType,
                   call->callToken);
     ooClearAllLogicalChannels(call);
  }
*strong*
{code}

BUT SHOULD BE:
{code:title=ooCalls.c|borderStyle=solid}
int ooEndCall(OOH323CallData *call)
{
  OOTRACEDBGA4("In ooEndCall call state is - %s (%s, %s)\n",
                ooGetCallStateText(call->callState), call->callType,
                call->callToken);

  if(call->callState == OO_CALL_REMOVED) {
     OOTRACEINFO2("Call already removed %s\n",
                   call->callToken);
    return OO_OK;
  }
+inserted+
  /* First clean all the logical channels, if not already cleaned. */
  if(call->logicalChans)
  {
     OOTRACEINFO3("Clearing all logical channels. (%s, %s)\n", call->callType,
                   call->callToken);
     ooClearAllLogicalChannels(call);
  }
+inserted+
  if (call->callIdentifier.guid.numocts == 0) call->callState = OO_CALL_CLEARED;

  if(!call->pH225Channel || call->pH225Channel->sock ==0)
  {
     call->callState = OO_CALL_CLEARED;
  }

  if(call->callState == OO_CALL_CLEARED || call->callState == OO_CALL_CLEAR_RELEASESENT)
  {
     ooCleanCall(call);
     call->callState = OO_CALL_REMOVED;
     return OO_OK;
  }
-deleted-
  if(call->logicalChans)
  {
     OOTRACEINFO3("Clearing all logical channels. (%s, %s)\n", call->callType,
                   call->callToken);
     ooClearAllLogicalChannels(call);
  }
-deleted-
{code}
By: Alexandr Dranchuk (dav) 2015-08-04 00:29:50.745-0500

here is tracelevel=6 for chan_ooh323 and "core set debug 9" for asterisk 11.18.0
By: Alexandr Dranchuk (dav) 2015-08-04 11:21:37.739-0500

UPDATE:
this fixes not all situations,
in case we get incoming call, we in some situations call just
configure_local_rtp(); and then setup_rtp_remote(); and we already get allocated RTP port
but if we get in dialplan Hangup() at this moment,
setup_rtp_connection() are not called! and in fact close_rtp_connection() not called as well.
logical channels not open at this moment also.

thus allocated RTP port not freed and rtp instance not destroyed. Not sure for correct way to clean up, and right
place.
May be in ooh323_destroy() call ast_rtp_instance_stop() anyway?
set for tests like that for now:
{code:title=chan_ooh323.c|borderStyle=solid}
int ooh323_destroy(struct ooh323_pvt *p)
{
...
if (cur->rtp) {
ast_rtp_instance_stop(cur->rtp); /*just do it as not always done*/
ast_rtp_instance_destroy(cur->rtp);
cur->rtp = NULL;
}
...
}
{code}

Will research for better solution

By: Alexander Anikin (may213) 2015-08-13 16:53:40.403-0500

Hi Alexander,

ooCleanCall called from ooEndCalls in all situations and was called also in your case.

>> 01:22:50:516  Cleaning Call (incoming, ooh323c_1)- reason:OO_REASON_REMOTE_CLEARED

ooClearAllLogicalChannels wasn't called here due to they was not started and that functions not affect to rtp instane and ports allocated for RTP.

Looks like you're right about ast_rtp_instance_destroy and i assume it's problem in asterisk rtp engine, I think if we destroy
rtp instance then allocated resource by this instance must free automatically.
But I will double check you suggestion.


By: Alexandr Dranchuk (dav) 2015-08-13 23:12:41.626-0500

Alexander thanks for your responce.
I forget to notice that all calls where made when call forwarded to SIP channel, thus asterisk was doing H323->SIP conversion.
(You can see this in attached logs)

With fixes I note before there are no more RTP port leaks. Tested for 10 days with more then 50000 live calls.

It would be great if You suggest any corrections and apply fixes to ooh323 trunk.
By: Alexandr Dranchuk (dav) 2015-08-14 05:36:54.733-0500

BTW
I think it's right that if we call ast_rtp_instance_destroy, then allocated resource by this instance must free automatically, without calling ast_rtp_instance_stop.

Should I report about it separately?
By: Alexander Anikin (may213) 2015-08-14 13:27:14.846-0500

from res/res_rtp_asterisk.c:

static int ast_rtp_destroy(struct ast_rtp_instance *instance)
{
...
       /* Close our own socket so we no longer get packets */
       if (rtp->s > -1) {
               close(rtp->s);
       }


same for rtcp socket.

Thus socket must be freed from instance destructor.
But ast_rtp_instance_destroy just act as decrement ref links to rtp instance and it's possible that there is not last reference to the rtp object.
And odd reference can be caused by OOH323 channel driver. I will research about references to rtp instance from chan_ooh323.


By: Alexander Anikin (may213) 2015-08-16 09:35:23.045-0500

Alexander,

I think it will mostly correct to insert ast_rtp_instance_stop before ast_rtp_instance_destroy
in current architecture of chan_ooh323 and asterisk rtp_engine.
Could you please attach here patch for that and test again with him?
If it will ok i will commit to asterisk repo.

By: Alexandr Dranchuk (dav) 2015-09-03 13:10:06.337-0500

Hi Alexander!
Having troubles with submiting patch thru gerrit, thus just put a diff file.
It will be great if you hint me with comments on correct way for patch preparation or contact me at alex.dranchuk@gmail.com
thanks!
By: Rusty Newton (rnewton) 2015-09-08 15:58:04.682-0500

[~dav] for future reference here is a link to the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

You can also ask for help with the patch contribution process on #asterisk-dev at irc.freenode.net or the asterisk-dev mailing list (http://lists.digium.com/mailman/listinfo/asterisk-dev).


By: Alexandr Dranchuk (dav) 2015-10-08 16:09:21.148-0500

Hi all!
Just noticed that asterisk 11.20.0.rc2 as well as asterisk 13.6.0rc2
has not full patch for this bag, i.e. file ooCalls.c doesn't contain the changes,
but chan_ooh323.c only.

Can it be fixed?
Thanks
By: Asterisk Team (asteriskteam) 2015-10-08 16:09:22.080-0500

This issue has been reopened as a result of your commenting on it as the reporter. It will be triaged once again as applicable.
By: Alexander Anikin (may213) 2017-11-03 19:02:34.038-0500

changes in ooCalls.c make no sense here and issue was solved by first part of patch only.