From 237d341bbd08168ca7b7bffd190125c032b57f39 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Tue, 27 Mar 2018 11:04:42 -0500 Subject: res_pjsip.c: Split ast_sip_push_task_synchronous() to fit expectations. ast_sip_push_task_synchronous() did not necessarily execute the passed in task under the specified serializer. If the current thread is any registered pjsip thread then it would execute the task immediately instead of under the specified serializer. Reentrancy issues could result if the task does not execute with the right serializer. The original reason ast_sip_push_task_synchronous() checked to see if the current thread was a registered pjsip thread was because of a deadlock with masquerades and the channel technology's fixup callback (ASTERISK_22936). A subsequent masquerade deadlock fix (ASTERISK_24356) involving call pickups avoided the original deadlock situation entirely. The PJSIP channel technology's fixup callback no longer needed to call ast_sip_push_task_synchronous(). However, there are a few places where this unexpected behavior is still required to avoid deadlocks. The pjsip monitor thread executes callbacks that do calls to ast_sip_push_task_synchronous() that would deadlock if the task were actually pushed to the specified serializer. I ran into one dealing with the pubsub subscriptions where an ao2 destructor called ast_sip_push_task_synchronous(). * Split ast_sip_push_task_synchronous() into ast_sip_push_task_wait_servant() and ast_sip_push_task_wait_serializer(). ast_sip_push_task_wait_servant() has the old behavior of ast_sip_push_task_synchronous(). ast_sip_push_task_wait_serializer() has the new behavior where the task is always executed by the specified serializer or a picked serializer if one is not passed in. Both functions behave the same if the current thread is not a SIP servant. * Redirected ast_sip_push_task_synchronous() to ast_sip_push_task_wait_servant() to preserve API for released branches. ASTERISK_26806 Change-Id: Id040fa42c0e5972f4c8deef380921461d213b9f3 --- channels/pjsip/dialplan_functions.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'channels/pjsip') diff --git a/channels/pjsip/dialplan_functions.c b/channels/pjsip/dialplan_functions.c index aa376f892..ce347dcd9 100644 --- a/channels/pjsip/dialplan_functions.c +++ b/channels/pjsip/dialplan_functions.c @@ -897,7 +897,7 @@ int pjsip_acf_channel_read(struct ast_channel *chan, const char *cmd, char *data func_args.field = args.field; func_args.buf = buf; func_args.len = len; - if (ast_sip_push_task_synchronous(func_args.session->serializer, read_pjsip, &func_args)) { + if (ast_sip_push_task_wait_serializer(func_args.session->serializer, read_pjsip, &func_args)) { ast_log(LOG_WARNING, "Unable to read properties of channel %s: failed to push task\n", ast_channel_name(chan)); ao2_ref(func_args.session, -1); return -1; @@ -1219,7 +1219,7 @@ int pjsip_acf_media_offer_write(struct ast_channel *chan, const char *cmd, char mdata.media_type = AST_MEDIA_TYPE_VIDEO; } - return ast_sip_push_task_synchronous(channel->session->serializer, media_offer_write_av, &mdata); + return ast_sip_push_task_wait_serializer(channel->session->serializer, media_offer_write_av, &mdata); } int pjsip_acf_dtmf_mode_read(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len) @@ -1390,7 +1390,7 @@ int pjsip_acf_dtmf_mode_write(struct ast_channel *chan, const char *cmd, char *d ast_channel_unlock(chan); - return ast_sip_push_task_synchronous(channel->session->serializer, dtmf_mode_refresh_cb, &rdata); + return ast_sip_push_task_wait_serializer(channel->session->serializer, dtmf_mode_refresh_cb, &rdata); } static int refresh_write_cb(void *obj) @@ -1438,5 +1438,5 @@ int pjsip_acf_session_refresh_write(struct ast_channel *chan, const char *cmd, c rdata.method = AST_SIP_SESSION_REFRESH_METHOD_UPDATE; } - return ast_sip_push_task_synchronous(channel->session->serializer, refresh_write_cb, &rdata); + return ast_sip_push_task_wait_serializer(channel->session->serializer, refresh_write_cb, &rdata); } -- cgit v1.2.3