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

George Joseph asteriskteam at digium.com
Wed Feb 24 13:52:33 CST 2021


Hello Joshua Colp, Friendly Automation, Richard Mudgett, 

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

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

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

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 its
duration and since pjproject also holds the dialog lock while
calling refer_progress_on_evsub_state() (which does the cleanup),
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, 51 insertions(+), 56 deletions(-)


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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I97a8bb01771a3c85345649b8124507f7622a8480
Gerrit-Change-Number: 15509
Gerrit-PatchSet: 4
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/5d87a6ec/attachment.html>


More information about the asterisk-code-review mailing list