[Asterisk-code-review] res_pjsip_refer: Refactor progress locking and serialization (asterisk[master])

[George Joseph]

Hello Joshua Colp, Friendly Automation, Richard Mudgett, 

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

    https://gerrit.asterisk.org/c/asterisk/+/15510

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

Change subject: res_pjsip_refer: Refactor progress locking and serialization
......................................................................

res_pjsip_refer: Refactor progress locking and serialization

Although refer_progress_notify() always runs in the progress
serializer, the pjproject evsub module itself can cause the
subscription to be destroyed which then triggers
refer_progress_on_evsub_state() to clean it up.  In this case,
it's possible that refer_progress_notify() could get the
subscription pulled out from under it while it's trying to use
it.

At one point we tried to have refer_progress_on_evsub_state()
push the cleanup to the serializer and wait for its return before
returning to pjproject but since pjproject calls its state
callbacks with the dialog locked, this required us to unlock the
dialog while waiting for the serialized cleanup, then lock it
again before returning to pjproject. There were also still some
cases where other callers of refer_progress_notify() weren't
using the serializer and crashes were resulting.

Although all callers of refer_progress_notify() now use the
progress serializer, we decided to simplify the locking so we
didn't have to unlock and relock the dialog in
refer_progress_on_evsub_state().

Now, refer_progress_notify() holds the dialog lock for all its
work rather than just when calling pjsip_evsub_set_mod_data() to
clear the module data.  Since pjproject also holds the dialog
lock while calling refer_progress_on_evsub_state(), there should
be no more chances for the subscription to be cleaned up while
still being used to send NOTIFYs.

ASTERISK-29313

Change-Id: I97a8bb01771a3c85345649b8124507f7622a8480
---
M res/res_pjsip_refer.c
1 file changed, 49 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/10/15510/3
-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15510
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I97a8bb01771a3c85345649b8124507f7622a8480
Gerrit-Change-Number: 15510
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: newpatchset
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210224/0b7385f8/attachment.html>