[Asterisk-code-review] res pjsip.c: Split ast sip push task synchronous() to fit ex... (asterisk[master])

[Richard Mudgett]

Hello Jenkins2, Joshua Colp, 

I'd like you to reexamine a change. Please visit

    https://gerrit.asterisk.org/8757

to look at the new patch set (#3).

Change subject: res_pjsip.c: Split ast_sip_push_task_synchronous() to fit expectations.
......................................................................

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
---
M channels/chan_pjsip.c
M channels/pjsip/dialplan_functions.c
M include/asterisk/res_pjsip.h
M res/res_pjsip.c
M res/res_pjsip/config_system.c
M res/res_pjsip/config_transport.c
M res/res_pjsip_header_funcs.c
M res/res_pjsip_history.c
M res/res_pjsip_outbound_publish.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_pubsub.c
M res/res_pjsip_refer.c
M res/res_pjsip_transport_websocket.c
13 files changed, 185 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/57/8757/3
-- 
To view, visit https://gerrit.asterisk.org/8757
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id040fa42c0e5972f4c8deef380921461d213b9f3
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 3
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180412/9d3931e3/attachment-0001.html>