Summary:ASTERISK-09309: [patch] Some PRI disconnect codes do not hangup correctly
Reporter:Tony Mountifield (softins)Labels:
Date Opened:2007-04-25 04:55:41Date Closed:2007-07-11 19:58:45
Versions:Frequency of
Environment:Attachments:( 0) chan_zap.c-1.2-diff.txt
( 1) chan_zap.c-1.2-diff3.txt
( 2) chan_zap.c-1.4-diff.txt
( 3) chan_zap.c-1.4-diff3.txt
( 4) chan_zap.c-trunk-diff.txt
( 5) chan_zap.c-trunk-diff2.txt
( 6) chan_zap.c-trunk-diff3.txt
Description:If the network sends a PRI disconnect to an established call using the cause codes listed for "congestion", the call will not be hung up properly. If the call was in a conference, the conference then hears a fault message from the network until the network does a forced hangup. In the UK this takes 40 seconds, which is quite disruptive!

This was observed when a caller on a mobile / cellphone lost signal and the disconnect cause code sent by the network was 41 (PRI_CAUSE_NORMAL_TEMPORARY_FAILURE).

In the handlers for PRI_EVENT_HANGUP and PRI_EVENT_HANGUP_REQ, the assumption is made that certain cause codes will only be received during call setup. I propose changing the logic so that AST_SOFTHANGUP_DEV will always be set if the call was UP, and otherwise needbusy or needcongestion will be set as appropriate:

if (pri->pvts[chanpos]->owner->_state == AST_STATE_UP)
 pri->pvts[chanpos]->owner->_softhangup |= AST_SOFTHANGUP_DEV;
else if (pri->pvts[chanpos]->owner->hangupcause == PRI_CAUSE_USER_BUSY)
 pri->pvts[chanpos]->subs[SUB_REAL].needbusy = 1;
 pri->pvts[chanpos]->subs[SUB_REAL].needcongestion = 1;

Patches for chan_zap.c are attached for 1.2 (61777), 1.4 (61779) and trunk (61784). They also include the cause code in the verbose log message.
Comments:By: Russell Bryant (russell) 2007-04-30 10:59:28

This patch has been merged into 1.2, 1.4, and trunk in revisions 62417, 62419, and 62422.  Thanks!

By: Russell Bryant (russell) 2007-05-23 08:11:42

This patch had to be reverted because of problems reported directly to Mark.  Unfortunately, the description was extremely vague.  We will have to readdress this issue, but I probably won't be able to look at it in detail until after this week.

By: James Texter III (jtexter3) 2007-05-23 13:03:36

I've attached chan_zap.c-trunk-diff2.txt, which I think has a lot fewer consequences for other disconnect causes, but should also solve the original problem.  I can also backport to 1.2 and 1.4 if that will help with testing.

By: Tony Mountifield (softins) 2007-05-23 13:26:17

OK, what would *really* help would be a concrete description of the reported problem with the original patch.

Looking at jtexter3's revised patch - it appears that would still fix my original problem.

I guess the questions that need to be answered are the following:

a) Can we agree that we should always set AST_SOFTHANGUP_DEV if the state is AST_STATE_UP? This was my fundamental issue, and is what was lacking in the original code prior to my patch.

b) Are there other states in which AST_SOFTHANGUP_DEV should be set, instead of setting needbusy or needcongestion? This was the possibility that I didn't consider before.

By: Tony Mountifield (softins) 2007-05-23 16:36:38

I've now uploaded revised versions of my patches that just do the soft hangup if the channel is up, but preserve the existing switch statement for all other states.

I think the resultant functionality is identical to jtexter3's patch, but it avoids having to re-test the needbusy and needcongestion flags.

Patches for 1.2, 1.4 and trunk, as chan_zap.c-*-diff3.txt, against current SVN.

By: mhardeman (mhardeman) 2007-05-23 19:16:38

I confess to being the person who reported the problem to Mark.  My apologies for not sending this in through the proper channels, but I happened to be talking to Mark and the issue came to mind as we were talking.

The problem observed by me was that if you had an inbound call coming in on the PRI and it was in a Dial to a SIP peer, then an outside user initiated disconnect would not tear down the SIP peer, resulting in funny behavior.  (i.e. Phone keeps ringing despite outsider having hung up.)

The last comment on this issue (by softins) prior to this one (mine) seems to indicate that someone figured out the core issue: considering states other than AST_STATE_UP.

I have not yet reviewed the patches referenced in that note, but if they reflect the behavior expressed in said comment then I'm pretty sure that's the proper approach.

Thanks and sorry again for not digging into this further and providing proper feedback earlier on.

By: Russell Bryant (russell) 2007-06-01 15:01:46

mhardeman: Could you please test these changes on your system to ensure that they don't cause you any problems?  Thank you

By: Russell Bryant (russell) 2007-06-20 13:48:01

The updated patch has been committed to 1.2, 1.4, and trunk in revisions 60396, 70397, and 70398.