| Summary: | ASTERISK-17211: [patch] AMI redirect from meetme - calls fail | ||||||
| Reporter: | Olle Johansson (oej) | Labels: | |||||
| Date Opened: | 2011-01-07 08:48:32.000-0600 | Date Closed: | 2011-01-25 11:27:02.000-0600 | ||||
| Priority: | Minor | Regression? | No | ||||
| Status: | Closed/Complete | Components: | Channels/General | ||||
| Versions: | Frequency of Occurrence | ||||||
| Related Issues: |
| ||||||
| Environment: | Attachments: | ( 0) ami-meetme.diff ( 1) ami-meetme2.diff ( 2) ami-meetme2v1.4.diff | |||||
| Description: | Possibly related to bug ASTERISK-16891 and review https://reviewboard.asterisk.org/r/1013/ Two calls in a meetme. Issue a redirect to get one call out to the dialplan - play a prompt there. The call hangs up with no prompt played. The PBX jumps to the dialplan and executes entries, but playback and wait fails If you have a normal call and a normal PBX bridge, it works. Testing with Asterisk 1.4 rev 300917 This has been working in earlier releases. I strongly suspect the changes in the above commits to have changed something. ****** ADDITIONAL INFORMATION ****** Have been adding millions of debug messages and it seems like poll returns with something to read, but waitstream_core returns after trying to read a frame and gets nothing... Dialplan for testing: [skrep] ; Transferring with AMI redirect to this extension from a meetme fails, no audio is played and ; something hangs up the call before reaching musiconhold exten => 1010,1,answer exten => 1010,n,playback(demo-congrats) exten => 1010,n,musiconhold [Jan 7 15:39:53] DEBUG[3228]: manager.c:2278 process_message: Manager received command 'redirect' [Jan 7 15:39:53] DEBUG[3228]: channel.c:1578 ast_softhangup_nolock: Soft-Hanging up channel 'SIP/pinefrog6-00000001' [Jan 7 15:39:53] DEBUG[3246]: pbx.c:2473 __ast_pbx_run: Spawn extension (pinefrog,1010,0) exited non-zero on 'SIP/pinefrog6-00000001' == Spawn extension (pinefrog, 1010, 0) exited non-zero on 'SIP/pinefrog6-00000001' [Jan 7 15:39:53] DEBUG[3246]: pbx.c:1866 pbx_extension_helper: Launching 'Answer' -- Executing [1010@pinefrog:1] Answer("SIP/pinefrog6-00000001", "") in new stack [Jan 7 15:39:53] DEBUG[3246]: pbx.c:1866 pbx_extension_helper: Launching 'Playback' -- Executing [1010@pinefrog:2] Playback("SIP/pinefrog6-00000001", "demo-congrats") in new stack [Jan 7 15:39:53] DEBUG[3246]: channel.c:3427 set_format: Set channel SIP/pinefrog6-00000001 to write format gsm [Jan 7 15:39:53] DEBUG[3246]: channel.c:2013 ast_settimeout: Scheduling timer at 160 sample intervals -- <SIP/pinefrog6-00000001> Playing 'demo-congrats' (language 'en') [Jan 7 15:39:53] DEBUG[3246]: channel.c:2013 ast_settimeout: Scheduling timer at 0 sample intervals [Jan 7 15:39:53] DEBUG[3246]: channel.c:2013 ast_settimeout: Scheduling timer at 0 sample intervals [Jan 7 15:39:53] DEBUG[3246]: channel.c:3427 set_format: Set channel SIP/pinefrog6-00000001 to write format slin [Jan 7 15:39:53] DEBUG[3246]: pbx.c:2473 __ast_pbx_run: Spawn extension (pinefrog,1010,2) exited non-zero on 'SIP/pinefrog6-00000001' == Spawn extension (pinefrog, 1010, 2) exited non-zero on 'SIP/pinefrog6-00000001' [Jan 7 15:39:53] DEBUG[3246]: channel.c:1578 ast_softhangup_nolock: Soft-Hanging up channel 'SIP/pinefrog6-00000001' [Jan 7 15:39:53] DEBUG[3246]: pbx.c:1866 pbx_extension_helper: Launching 'NoOp' -- Executing [h@pinefrog:1] NoOp("SIP/pinefrog6-00000001", "----HANGUP channel: ") in new stack exten => h,1,noop(----HANGUP channel: ${HANGUPCHANNEL} ) exten => h,n,dumpchan() exten => _3X.,1,noop(TEST extension: ${EXTEN}) exten => _3X.,n,meetme(3000,qdx) exten => _3X.,n,hangup() | ||||||
| Comments: | By: Olle Johansson (oej) 2011-01-07 08:49:44.000-0600 Linux Centos 5 with DAHDI install as a timer device. By: Olle Johansson (oej) 2011-01-10 03:13:10.000-0600 Confirmed different behaviour between 1.4 rev 288636 and the version with this patch. By: Olle Johansson (oej) 2011-01-10 04:44:12.000-0600 1.4 rev 295790 introduces the reported problem. By: Damien Wedhorn (wedhorn) 2011-01-10 20:18:15.000-0600 Had a play and found the issue is introduced in r291577. Notably, tested with skinny devices, but r291392 plays demo-congrats when AMI redirect from Meetme, whereas r291577 hangs up. The additional hangup in 291577 is obvious (and arguably correct). Looks like AST_SOFTHANGUP_ASYNCGOTO needs to be cleared before the next frame is read. Or.... Look at removing AST_SOFTHANGUP_ASYNCGOTO. It looks like it's set in ast_async_goto, and unset in collect_digits and avoids an error state (only for the first digit), and unset in __ast_pbx_run and avoids error states. However, given that it is only set when there is a pbx, unsetting when initiating a pbx would probably never occur. By: Damien Wedhorn (wedhorn) 2011-01-11 03:58:16.000-0600 ami-meetme.diff added. Fixes the issue for me. It's a hack, but highlights the issue. By: Olle Johansson (oej) 2011-01-11 05:36:33.000-0600 I can confirm that wedhorn's patch works. I can not judge whether it's the proper patch or not, but it solved my problem. Thanks, wedhorn. By: Damien Wedhorn (wedhorn) 2011-01-12 17:23:30.000-0600 Two patches uploaded (ami-meetme2.diff and one for 1.4). Issue seems to exist in all branches. Also applies to cli_redirect when channel in app_meetme. This patch seems to fix the issues and I've no issues with committing after testing. Tested with redirecting single skinny device from app_meetme to say with ami and cli. By: Olle Johansson (oej) 2011-01-13 01:15:32.000-0600 Was there a good reason for checking for hangups there? By: Damien Wedhorn (wedhorn) 2011-01-13 03:42:26.000-0600 I can't think of one, except that at face value it seems reasonable. Why keep the queue going if someone is hanging up? That's why I'd suggest a review. Personally I think it should go because there is no way that app_meetme coders would be aware of all the ins and outs of who's changing what to what in the softhangup bitmask. Nor should they need to be. The issue does however indicate issues with ast_chan_hangup. Maybe it should be renamed to ast_chan_maybe_maybenot_hangup. Actually maybe a better name would be ast_chan_softhangup_bitmask_in_use. By: Russell Bryant (russell) 2011-01-20 17:16:11.000-0600 It took me a bit to understand how removing ast_check_hangup() fixed it, but once I figured it out, I made a change at a deeper level to solve the issue. Basically, softhangup was being set as intended which causes app_meetme to exit when desired, but there was code that aborts the hangup process in the case of an async goto which did not fully take into account some newer changes to how a softhangup is processed. https://reviewboard.asterisk.org/r/1082/ By: Olle Johansson (oej) 2011-01-22 03:29:04.000-0600 Thank you, Russell! I will take a look and test. I realized yesterday that this also affects normal Meetme use. After returning to the dialplan the call dies... I tested this by having one unmarked user and one marked user - with the option "exit when the last marked user leaves conference". There was no way to continue in the dial plan. By: Olle Johansson (oej) 2011-01-22 03:30:50.000-0600 ...and this happened in code with BOTH patches by wedhorn. By: Digium Subversion (svnbot) 2011-01-24 14:32:23.000-0600 Repository: asterisk Revision: 303546 U branches/1.4/apps/app_meetme.c U branches/1.4/include/asterisk/channel.h U branches/1.4/main/channel.c U branches/1.4/main/pbx.c U branches/1.4/res/res_features.c ------------------------------------------------------------------------ r303546 | russell | 2011-01-24 14:32:23 -0600 (Mon, 24 Jan 2011) | 31 lines Fix channel redirect out of MeetMe() and other issues with channel softhangup. Mantis issue ASTERISK-17211 reports that a channel redirect out of MeetMe() stopped working properly. This issue includes a patch that resolves the issue by removing a call to ast_check_hangup() from app_meetme.c. I left that in my patch, as it doesn't need to be there. However, the rest of the patch fixes this problem with or without the change to app_meetme. The key difference between what happens before and after this patch is the effect of the END_OF_Q control frame. After END_OF_Q is hit in ast_read(), ast_read() will return NULL. With the ast_check_hangup() removed, app_meetme sees this which causes it to exit as intended. Checking ast_check_hangup() caused app_meetme to exit earlier in the process, and the target of the redirect saw the condition where ast_read() returned NULL. Removing ast_check_hangup() works around the issue in app_meetme, but doesn't solve the issue if another application did the same thing. There are also other edge cases where if an application finishes at the same time that a redirect happens, the target of the redirect will think that the channel hung up. So, I made some changes in pbx.c to resolve it at a deeper level. There are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to abort the hangup process. My patch extends this to remove the END_OF_Q frame from the channel's read queue, making the "abort hangup" more complete. This same technique was used in every place where a softhangup flag was cleared. (closes issue ASTERISK-17211) Reported by: oej Tested by: oej, wedhorn, russell Review: https://reviewboard.asterisk.org/r/1082/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=303546 By: Digium Subversion (svnbot) 2011-01-24 14:49:54.000-0600 Repository: asterisk Revision: 303548 _U branches/1.6.2/ U branches/1.6.2/apps/app_meetme.c U branches/1.6.2/include/asterisk/channel.h U branches/1.6.2/main/channel.c U branches/1.6.2/main/features.c U branches/1.6.2/main/pbx.c ------------------------------------------------------------------------ r303548 | russell | 2011-01-24 14:49:54 -0600 (Mon, 24 Jan 2011) | 38 lines Merged revisions 303546 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r303546 | russell | 2011-01-24 14:32:21 -0600 (Mon, 24 Jan 2011) | 31 lines Fix channel redirect out of MeetMe() and other issues with channel softhangup. Mantis issue ASTERISK-17211 reports that a channel redirect out of MeetMe() stopped working properly. This issue includes a patch that resolves the issue by removing a call to ast_check_hangup() from app_meetme.c. I left that in my patch, as it doesn't need to be there. However, the rest of the patch fixes this problem with or without the change to app_meetme. The key difference between what happens before and after this patch is the effect of the END_OF_Q control frame. After END_OF_Q is hit in ast_read(), ast_read() will return NULL. With the ast_check_hangup() removed, app_meetme sees this which causes it to exit as intended. Checking ast_check_hangup() caused app_meetme to exit earlier in the process, and the target of the redirect saw the condition where ast_read() returned NULL. Removing ast_check_hangup() works around the issue in app_meetme, but doesn't solve the issue if another application did the same thing. There are also other edge cases where if an application finishes at the same time that a redirect happens, the target of the redirect will think that the channel hung up. So, I made some changes in pbx.c to resolve it at a deeper level. There are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to abort the hangup process. My patch extends this to remove the END_OF_Q frame from the channel's read queue, making the "abort hangup" more complete. This same technique was used in every place where a softhangup flag was cleared. (closes issue ASTERISK-17211) Reported by: oej Tested by: oej, wedhorn, russell Review: https://reviewboard.asterisk.org/r/1082/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=303548 By: Digium Subversion (svnbot) 2011-01-24 14:51:38.000-0600 Repository: asterisk Revision: 303549 _U branches/1.8/ U branches/1.8/apps/app_meetme.c U branches/1.8/include/asterisk/channel.h U branches/1.8/main/channel.c U branches/1.8/main/features.c U branches/1.8/main/pbx.c ------------------------------------------------------------------------ r303549 | russell | 2011-01-24 14:51:38 -0600 (Mon, 24 Jan 2011) | 45 lines Merged revisions 303548 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ................ r303548 | russell | 2011-01-24 14:49:53 -0600 (Mon, 24 Jan 2011) | 38 lines Merged revisions 303546 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r303546 | russell | 2011-01-24 14:32:21 -0600 (Mon, 24 Jan 2011) | 31 lines Fix channel redirect out of MeetMe() and other issues with channel softhangup. Mantis issue ASTERISK-17211 reports that a channel redirect out of MeetMe() stopped working properly. This issue includes a patch that resolves the issue by removing a call to ast_check_hangup() from app_meetme.c. I left that in my patch, as it doesn't need to be there. However, the rest of the patch fixes this problem with or without the change to app_meetme. The key difference between what happens before and after this patch is the effect of the END_OF_Q control frame. After END_OF_Q is hit in ast_read(), ast_read() will return NULL. With the ast_check_hangup() removed, app_meetme sees this which causes it to exit as intended. Checking ast_check_hangup() caused app_meetme to exit earlier in the process, and the target of the redirect saw the condition where ast_read() returned NULL. Removing ast_check_hangup() works around the issue in app_meetme, but doesn't solve the issue if another application did the same thing. There are also other edge cases where if an application finishes at the same time that a redirect happens, the target of the redirect will think that the channel hung up. So, I made some changes in pbx.c to resolve it at a deeper level. There are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to abort the hangup process. My patch extends this to remove the END_OF_Q frame from the channel's read queue, making the "abort hangup" more complete. This same technique was used in every place where a softhangup flag was cleared. (closes issue ASTERISK-17211) Reported by: oej Tested by: oej, wedhorn, russell Review: https://reviewboard.asterisk.org/r/1082/ ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=303549 By: Digium Subversion (svnbot) 2011-01-24 14:57:29.000-0600 Repository: asterisk Revision: 303551 _U trunk/ U trunk/apps/app_meetme.c U trunk/include/asterisk/channel.h U trunk/main/channel.c U trunk/main/features.c U trunk/main/pbx.c ------------------------------------------------------------------------ r303551 | russell | 2011-01-24 14:57:29 -0600 (Mon, 24 Jan 2011) | 52 lines Merged revisions 303549 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................ r303549 | russell | 2011-01-24 14:51:37 -0600 (Mon, 24 Jan 2011) | 45 lines Merged revisions 303548 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ................ r303548 | russell | 2011-01-24 14:49:53 -0600 (Mon, 24 Jan 2011) | 38 lines Merged revisions 303546 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r303546 | russell | 2011-01-24 14:32:21 -0600 (Mon, 24 Jan 2011) | 31 lines Fix channel redirect out of MeetMe() and other issues with channel softhangup. Mantis issue ASTERISK-17211 reports that a channel redirect out of MeetMe() stopped working properly. This issue includes a patch that resolves the issue by removing a call to ast_check_hangup() from app_meetme.c. I left that in my patch, as it doesn't need to be there. However, the rest of the patch fixes this problem with or without the change to app_meetme. The key difference between what happens before and after this patch is the effect of the END_OF_Q control frame. After END_OF_Q is hit in ast_read(), ast_read() will return NULL. With the ast_check_hangup() removed, app_meetme sees this which causes it to exit as intended. Checking ast_check_hangup() caused app_meetme to exit earlier in the process, and the target of the redirect saw the condition where ast_read() returned NULL. Removing ast_check_hangup() works around the issue in app_meetme, but doesn't solve the issue if another application did the same thing. There are also other edge cases where if an application finishes at the same time that a redirect happens, the target of the redirect will think that the channel hung up. So, I made some changes in pbx.c to resolve it at a deeper level. There are already places that unset the SOFTHANGUP_ASYNCGOTO flag in an attempt to abort the hangup process. My patch extends this to remove the END_OF_Q frame from the channel's read queue, making the "abort hangup" more complete. This same technique was used in every place where a softhangup flag was cleared. (closes issue ASTERISK-17211) Reported by: oej Tested by: oej, wedhorn, russell Review: https://reviewboard.asterisk.org/r/1082/ ........ ................ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=303551 | ||||||