From 3c7dde3bf6592af3a81f55815f956f7a9dae1ea9 Mon Sep 17 00:00:00 2001 From: Naveen Albert Date: Sat, 24 Sep 2022 10:15:09 +0000 Subject: [PATCH] sla: Prevent deadlock and crash due to autoservicing. SLAStation currently autoservices the station channel before creating a thread to actually dial the trunk. This leads to duplicate servicing of the channel which causes assertions, deadlocks, crashes, and moreover not the correct behavior. Removing the autoservice prevents the crash, but if the station hangs up before the trunk answers, the call hangs since the hangup was never serviced on the channel. This is fixed by not autoservicing the channel, but instead servicing it in the thread dialing the trunk, since it is doing so synchronously to begin with. Instead of sleeping for 100ms in a loop, we simply use the channel for timing, and abort if it disappears. The same issue also occurs with SLATrunk when a call is answered, because ast_answer invokes ast_waitfor_nandfds. Thus, we use ast_raw_answer instead which does not cause any conflict and allows the call to be answered normally without thread blocking issues. ASTERISK-29998 #close Change-Id: Icc237d50354b5910000d2305901e86d2c87bb9d8 --- apps/app_meetme.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/apps/app_meetme.c b/apps/app_meetme.c index 191de68ae7..8a98fd9941 100644 --- a/apps/app_meetme.c +++ b/apps/app_meetme.c @@ -6149,7 +6149,7 @@ struct run_station_args { static void answer_trunk_chan(struct ast_channel *chan) { - ast_answer(chan); + ast_raw_answer(chan); ast_indicate(chan, -1); } @@ -6994,8 +6994,18 @@ static void *dial_trunk(void *data) return NULL; } - for (;;) { + /* Wait for dial to end, while servicing the channel */ + while (ast_waitfor(trunk_ref->chan, 100)) { unsigned int done = 0; + struct ast_frame *fr = ast_read(trunk_ref->chan); + + if (!fr) { + ast_debug(1, "Channel %s did not return a frame, must have hung up\n", ast_channel_name(trunk_ref->chan)); + done = 1; + break; + } + ast_frfree(fr); /* Ignore while dialing */ + switch ((dial_res = ast_dial_state(dial))) { case AST_DIAL_RESULT_ANSWERED: trunk_ref->trunk->chan = ast_dial_answered(dial); @@ -7032,8 +7042,6 @@ static void *dial_trunk(void *data) last_state = current_state; } - /* avoid tight loop... sleep for 1/10th second */ - ast_safe_sleep(trunk_ref->chan, 100); } if (!trunk_ref->trunk->chan) { @@ -7192,8 +7200,10 @@ static int sla_station_exec(struct ast_channel *chan, const char *data) sla_change_trunk_state(trunk_ref->trunk, SLA_TRUNK_STATE_UP, ALL_TRUNK_REFS, NULL); /* Create a thread to dial the trunk and dump it into the conference. * However, we want to wait until the trunk has been dialed and the - * conference is created before continuing on here. */ - ast_autoservice_start(chan); + * conference is created before continuing on here. + * Don't autoservice the channel or we'll have multiple threads + * handling it. dial_trunk services the channel. + */ ast_mutex_init(&cond_lock); ast_cond_init(&cond, NULL); ast_mutex_lock(&cond_lock); @@ -7202,7 +7212,7 @@ static int sla_station_exec(struct ast_channel *chan, const char *data) ast_mutex_unlock(&cond_lock); ast_mutex_destroy(&cond_lock); ast_cond_destroy(&cond); - ast_autoservice_stop(chan); + if (!trunk_ref->trunk->chan) { ast_debug(1, "Trunk didn't get created. chan: %lx\n", (unsigned long) trunk_ref->trunk->chan); pbx_builtin_setvar_helper(chan, "SLASTATION_STATUS", "CONGESTION");