From: Alexei Gradinari Date: Sat, 6 Aug 2016 15:57:08 +0000 (-0400) Subject: pjsip: Fix deadlock with suspend taskprocessor on masquerade X-Git-Tag: 13.12.0-rc1~148^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1589452fdc044dc6233e73e87c0e3735c97a5495;p=thirdparty%2Fasterisk.git pjsip: Fix deadlock with suspend taskprocessor on masquerade If both channels which should be masqueraded are in the same serializer: 1st channel will be locked waiting condition 'complete' 2nd channel will be locked waiting condition 'suspended' On heavy load system a chance that both channels will be in the same serializer 'pjsip/distibutor' is very high. To reproduce compile res_pjsip/pjsip_distributor.c with DISTRIBUTOR_POOL_SIZE=1 Steps to reproduce: 1. Party A calls Party B (bridged call 'AB') 2. Party B places Party A on hold 3. Party B calls Voicemail app (non-bridged call 'BV') 4. Party B attended transfers Party A to voicemail using REFER. 5. When asterisk masquerades calls 'AB' and 'BV', a deadlock is happened. This patch adds a suspension indicator to the taskprocessor. When a session suspends/unsuspends the serializer it sets the indicator to the appropriate state. The session checks the suspension indicator before suspend the serializer. ASTERISK-26145 #close Change-Id: Iaaebee60013a58c942ba47b1b4930a63e686663b --- diff --git a/include/asterisk/taskprocessor.h b/include/asterisk/taskprocessor.h index e51122269d..7c79036b37 100644 --- a/include/asterisk/taskprocessor.h +++ b/include/asterisk/taskprocessor.h @@ -241,6 +241,38 @@ struct ast_taskprocessor_local { int ast_taskprocessor_push_local(struct ast_taskprocessor *tps, int (*task_exe)(struct ast_taskprocessor_local *local), void *datap); +/*! + * \brief Indicate the taskprocessor is suspended. + * + * \since 13.12.0 + * + * \param tps Task processor. + * \retval 0 success + * \retval -1 failure + */ +int ast_taskprocessor_suspend(struct ast_taskprocessor *tps); + +/*! + * \brief Indicate the taskprocessor is unsuspended. + * + * \since 13.12.0 + * + * \param tps Task processor. + * \retval 0 success + * \retval -1 failure + */ +int ast_taskprocessor_unsuspend(struct ast_taskprocessor *tps); + +/*! + * \brief Get the task processor suspend status + * + * \since 13.12.0 + * + * \param tps Task processor. + * \retval non-zero if the task processor is suspended + */ +int ast_taskprocessor_is_suspended(struct ast_taskprocessor *tps); + /*! * \brief Pop a task off the taskprocessor and execute it. * diff --git a/main/taskprocessor.c b/main/taskprocessor.c index 7ce3e4f16f..0c429cf3f7 100644 --- a/main/taskprocessor.c +++ b/main/taskprocessor.c @@ -91,6 +91,8 @@ struct ast_taskprocessor { unsigned int high_water_warned:1; /*! Indicates that a high water alert is active on this taskprocessor */ unsigned int high_water_alert:1; + /*! Indicates if the taskprocessor is currently suspended */ + unsigned int suspended:1; }; /*! @@ -910,6 +912,33 @@ int ast_taskprocessor_push_local(struct ast_taskprocessor *tps, int (*task_exe)( return taskprocessor_push(tps, tps_task_alloc_local(task_exe, datap)); } +int ast_taskprocessor_suspend(struct ast_taskprocessor *tps) +{ + if (tps) { + ao2_lock(tps); + tps->suspended = 1; + ao2_unlock(tps); + return 0; + } + return -1; +} + +int ast_taskprocessor_unsuspend(struct ast_taskprocessor *tps) +{ + if (tps) { + ao2_lock(tps); + tps->suspended = 0; + ao2_unlock(tps); + return 0; + } + return -1; +} + +int ast_taskprocessor_is_suspended(struct ast_taskprocessor *tps) +{ + return tps ? tps->suspended : -1; +} + int ast_taskprocessor_execute(struct ast_taskprocessor *tps) { struct ast_taskprocessor_local local; diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c index 4290f6854e..a41e0aa5d0 100644 --- a/res/res_pjsip_session.c +++ b/res/res_pjsip_session.c @@ -1559,6 +1559,11 @@ void ast_sip_session_suspend(struct ast_sip_session *session) return; } + if (ast_taskprocessor_is_suspended(session->serializer)) { + /* The serializer already suspended. */ + return; + } + suspender = ao2_alloc(sizeof(*suspender), sip_session_suspender_dtor); if (!suspender) { /* We will just have to hope that the system does not deadlock */ @@ -1583,6 +1588,8 @@ void ast_sip_session_suspend(struct ast_sip_session *session) ast_cond_wait(&suspender->cond_suspended, ao2_object_get_lockaddr(suspender)); } ao2_unlock(suspender); + + ast_taskprocessor_suspend(session->serializer); } void ast_sip_session_unsuspend(struct ast_sip_session *session) @@ -1602,6 +1609,8 @@ void ast_sip_session_unsuspend(struct ast_sip_session *session) ao2_unlock(suspender); ao2_ref(suspender, -1); + + ast_taskprocessor_unsuspend(session->serializer); } /*!