[Asterisk-code-review] res pjsip distributor: Get a reference to dialog serializer. (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: res_pjsip_distributor: Get a reference to dialog serializer.
......................................................................


Patch Set 1:

This situation may actually be impossible with sessions, but I think it's possible to prove with subscriptions.

The only reference to the serializer is owned by the subscription tree, so when the subscription tree is destroyed, the serializer is destroyed as well.

The ownership model for subscriptions is that the pjsip_evsub owns a reference, and each resource that is being subscribed to owns a reference. In the case of most subscriptions, there is only one resource being subscribed to, so there are a total of two owners of the subscription tree.

During shutdown, the following might occur:
* A dialplan hint is removed, resulting in res_pjsip_exten_state being told to kill a subscription
* res_pjsip_exten_state queues a task on the subscription serializer to send the final NOTIFY.
* The serialized task executes, locking the dialog so that a NOTIFY can be sent.
* At around the same time, an incoming request on the dialog arrives (perhaps a subscription renewal). It now blocks attempting to lock the dialog inside the distributor code.
* The NOTIFY is sent on the serializer thread, with the dialog locked.
* The NOTIFY results in the pubsub_on_evsub_state() callback being called. Since we are sending our final NOTIFY on the subscription, we remove the pjsip_evsub reference to the subscription tree.
* Once the NOTIFY is sent, the dialog is unlocked.
* The incoming SUBSCRIBE request grabs the dialog lock within the distributor and gets a pointer to the serializer.
* res_pjsip_exten_state, having sent its NOTIFY, now drops its final reference to the subscription tree, resulting in its destruction.
* The subscription is destroyed, and with it, the serializer.
* The distributor now attempts to push a task on to the destroyed serializer. BOOM.

I don't necessarily know that this is what was happening on the crash that I saw, but it does illustrate how the distributor can attempt to push a task onto a destroyed serializer. It may also be possible for something similar to happen with sessions, but that is much harder to prove.

With the fix in this patch, if the above scenario happened, then the distributor would be able to successfully push the task onto the serializer. The incoming request would either be dropped by the pjsip_evsub layer if the subscription is destroyed already there, or we would ignore the request if it reached us since the pjsip_evsub's reference to the subscription tree would be gone already.

-- 
To view, visit https://gerrit.asterisk.org/1284
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I14563093f8320f79063b65792461f18f06c07a77
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No