| Summary: | ASTERISK-24234: app_meetme: Crash on conference shutdown due to NULL channel passed to meetme_stasis_generate_msg() | ||||
| Reporter: | Shaun Ruffell (sruffell) | Labels: | |||
| Date Opened: | 2014-08-14 11:17:18 | Date Closed: | 2014-08-17 18:38:16 | ||
| Priority: | Major | Regression? | Yes | ||
| Status: | Closed/Complete | Components: | Applications/app_meetme | ||
| Versions: | 13.0.0-beta1 | Frequency of Occurrence | Frequent | ||
| Related Issues: |
| ||||
| Environment: | Attachments: | ( 0) 0001-channels-Do-not-crash-if-NULL-channel-passed-to-ast_.patch ( 1) ASTERISK-24234-12-meetme.diff | |||
| Description: | Updated from an old version of 12 to 13-beta1 last night. Called into a meetme conference and when I left the conference asterisk crashed.
The backtrace: {noformat} #0 in ast_channel_uniqueid (chan=0x0) at channel_internal_api.c:489 #1 in meetme_stasis_generate_msg (meetme_conference=0xb724b5e8, chan=0x0, user=0x0, message_type=0x85a6afc, extras=0x0) at app_meetme.c:1383 #2 in conf_free (conf=0xb724b5e8) at app_meetme.c:2326 #3 in dispose_conf (conf=0xb724b5e8) at app_meetme.c:2503 #4 in conf_exec (chan=0xb7ef50ec, data=0xb738cbe8 "1337,i") at app_meetme.c:5138 #5 in pbx_exec (c=0xb7ef50ec, app=0x85a7d38, data=0xb738cbe8 "1337,i") at pbx.c:1658 #6 in pbx_extension_helper (c=0xb7ef50ec, con=0x0, context=0xb7ef5774 "phones", exten=0xb7ef57c4 "1337", priority=5, label=0x0, callerid=0xb7e759c8 "6001", action=E_SPAWN, found=0xb738f060, combined_find_spawn=1) at pbx.c:4930 #7 in ast_spawn_extension (c=0xb7ef50ec, context=0xb7ef5774 "phones", exten=0xb7ef57c4 "1337", priority=5, callerid=0xb7e759c8 "6001", found=0xb738f060, combined_find_spawn=1) at pbx.c:5945 {noformat} | ||||
| Comments: | By: Shaun Ruffell (sruffell) 2014-08-14 11:21:57.987-0500 I got things working at home with patch [^0001-channels-Do-not-crash-if-NULL-channel-passed-to-ast_.patch] but it doesn't seem right. I pinged #asterisk-dev and mjordan had this conversation with me: {noformat} sruffell My basic question is what should meetme_stasis_generate_msg() emit when the channel is NULL. mjordan 11:03 looks 11:04 hm 11:04 that function is documented as being NULL tolerant, but clearly is not 11:04 I think when we went through and did the performance improvements, we messed that up. sruffell 11:05 it looks like the null tollerance was removed in commit 404260 11:05 arghh..wait..wrong revision 11:05 404260: https://github.com/sruffell/asterisk-working/commit/8d70015cdd4 mjordan 11:06 Hm. 11:06 well, that would crash as well 11:06 ast_channel_blob_create is null tolerant 11:06 ast_channel_blob_create_from_cache is not, however 11:07 neither is grabbing ast_channel_uniqueid 11:07 it should just use ast_channel_blob_create. You have to use that one if you allow for NULL channels. mjordan 11:07 sruffell: mind making an issue? sruffell 11:07 nope..not at all…will do. mjordan 11:08 sruffell: ast_channel_blob_create creates a 'snapshot' of a channel, which is a relatively expensive thing to do. If what you want is a snapshot but you know the state of the channel hasn't changed, you can get it from the cache mjordan 11:09 in this case, we know that the state of the channel hasn't changed; the state of MeetMe has. However, this method gets called with and without a channel... and ast_channel_blob_create does more than just pack a channel into something that can be delivered over Stasis 11:09 ast_channel_topic is also NULL safe sruffell 11:10 hmm..ok. mjordan 11:10 sruffell: the locking if the channel is present is probably still necessary, so kharwell's patch should be restored, but should check for chan being NULL before locking/unlocking. mjordan 11:10 (If you felt like writing a patch :-) ) sruffell 11:11 :) I will. I had written this one: https://github.com/sruffell/asterisk-working/commit/bf6bddcc7963f3aacc223108b36dc6501 in order to get things working, just didn't seem like the right thing. But it's what I running at home right now (beta-1 + that patch) mjordan 11:15 sruffell: heh. That would do interesting things :-) {noformat} So I'll see about updating the patch to use and testing it out tonight. By: Matt Jordan (mjordan) 2014-08-14 14:38:15.967-0500 Give this patch a try. It was written against Asterisk 12, but this issue should exist in the latest Asterisk 12 as well as Asterisk 13 beta1. By: Shaun Ruffell (sruffell) 2014-08-14 16:02:07.980-0500 [^ASTERISK-24234-12-meetme.diff] applied on r421042 resolves the crash I was seeing. Thanks! By: Mitch Claborn (mclaborn) 2014-08-22 18:07:50.140-0500 Patch also works for me on 12.5. | ||||