res_pjsip_pubsub: Prevent sending NOTIFY on destroyed dialog.

A certain situation can result in our attempting to send a NOTIFY on a
destroyed dialog. Say we attempt to send a NOTIFY to a subscriber, but
that subscriber has dropped off the network. We end up retransmitting
that NOTIFY until the appropriate SIP timer says to destroy the NOTIFY
transaction. When the pjsip evsub code is told that the transaction has
been terminated, it responds in kind by alerting us that the
subscription has been terminated, destroying the subscription, and then
removing its reference to the dialog, thus destroying the dialog.

The problem is that when we get told that the subscription is being
terminated, we detect that we have not sent a terminating NOTIFY
request, so we queue up such a NOTIFY to be sent out. By the time that
queued NOTIFY gets sent, the dialog has been destroyed, so attempting to
send that NOTIFY can result in a crash.

The fix being introduced here is actually a reintroduction of something
the pubsub code used to employ. We hold a reference to the dialog and
wait to decrement our reference to the dialog until our subscription
tree object is destroyed. This way, we can send messages on the dialog
even if the PJSIP evsub code wants to terminate earlier than we would
like.

In doing this, some NULL checks for subscription tree dialogs have been
removed since NULL dialogs are no longer actually possible.

Change-Id: I013f43cddd9408bb2a31b77f5db87a7972bfe1e5

diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index 94125b9a3a418d6e130b934af82a31ef9ab35334..8d3ea6f5cf11f2f0dee459a0a3e7d01c353b910b 100644 (file)
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -1172,6 +1172,24 @@ static void shutdown_subscriptions(struct ast_sip_subscription *sub)
                sub->handler->subscription_shutdown(sub);
        }
 }
+static int subscription_unreference_dialog(void *obj)
+{
+       struct sip_subscription_tree *sub_tree = obj;
+
+       /* This is why we keep the dialog on the subscription. When the subscription
+        * is destroyed, there is no guarantee that the underlying dialog is ready
+        * to be destroyed. Furthermore, there's no guarantee in the opposite direction
+        * either. The dialog could be destroyed before our subscription is. We fix
+        * this problem by keeping a reference to the dialog until it is time to
+        * destroy the subscription. We need to have the dialog available when the
+        * subscription is destroyed so that we can guarantee that our attempt to
+        * remove the serializer will be successful.
+        */
+       pjsip_dlg_dec_session(sub_tree->dlg, &pubsub_module);
+       sub_tree->dlg = NULL;
+
+       return 0;
+}
 
 static void subscription_tree_destructor(void *obj)
 {
@@ -1185,6 +1203,7 @@ static void subscription_tree_destructor(void *obj)
 
        destroy_subscriptions(sub_tree->root);
 
+       ast_sip_push_task_synchronous(sub_tree->serializer, subscription_unreference_dialog, sub_tree);
        ast_taskprocessor_unreference(sub_tree->serializer);
        ast_module_unref(ast_module_info->self);
 }
@@ -1201,6 +1220,7 @@ static void subscription_setup_dialog(struct sip_subscription_tree *sub_tree, pj
        ast_sip_dialog_set_serializer(dlg, sub_tree->serializer);
        ast_sip_dialog_set_endpoint(dlg, sub_tree->endpoint);
        pjsip_evsub_set_mod_data(sub_tree->evsub, pubsub_module.id, sub_tree);
+       pjsip_dlg_inc_session(dlg, &pubsub_module);
 }
 
 static struct sip_subscription_tree *allocate_subscription_tree(struct ast_sip_endpoint *endpoint)
@@ -1521,10 +1541,6 @@ void *ast_sip_subscription_get_header(const struct ast_sip_subscription *sub, co
        pj_str_t name;
 
        dlg = sub->tree->dlg;
-       if (!dlg) {
-               return NULL;
-       }
-
        msg = ast_sip_mod_data_get(dlg->mod_data, pubsub_module.id, MOD_DATA_MSG);
        pj_cstr(&name, header);
 
@@ -2124,10 +2140,6 @@ static int serialized_send_notify(void *userdata)
        struct sip_subscription_tree *sub_tree = userdata;
        pjsip_dialog *dlg = sub_tree->dlg;
 
-       if (!dlg) {
-               return 0;
-       }
-
        pjsip_dlg_inc_lock(dlg);
        /* It's possible that between when the notification was scheduled
         * and now, that a new SUBSCRIBE arrived, requiring full state to be
@@ -2181,10 +2193,6 @@ int ast_sip_subscription_notify(struct ast_sip_subscription *sub, struct ast_sip
        int res;
        pjsip_dialog *dlg = sub->tree->dlg;
 
-       if (!dlg) {
-               return 0;
-       }
-
        pjsip_dlg_inc_lock(dlg);
 
        if (!sub->tree->evsub) {
@@ -2229,10 +2237,6 @@ void ast_sip_subscription_get_remote_uri(struct ast_sip_subscription *sub, char
        pjsip_dialog *dlg;
 
        dlg = sub->tree->dlg;
-       if (!dlg) {
-               *buf = '\0';
-               return;
-       }
        ast_copy_pj_str(buf, &dlg->remote.info_str, size);
 }
 
@@ -3189,10 +3193,6 @@ static int serialized_pubsub_on_server_timeout(void *userdata)
        struct sip_subscription_tree *sub_tree = userdata;
        pjsip_dialog *dlg = sub_tree->dlg;
 
-       if (!dlg) {
-               return 0;
-       }
-
        pjsip_dlg_inc_lock(dlg);
        if (!sub_tree->evsub) {
                pjsip_dlg_dec_lock(dlg);
@@ -3281,7 +3281,6 @@ static void pubsub_on_evsub_state(pjsip_evsub *evsub, pjsip_event *event)
        sub_tree->evsub = NULL;
        ast_sip_dialog_set_serializer(sub_tree->dlg, NULL);
        ast_sip_dialog_set_endpoint(sub_tree->dlg, NULL);
-       sub_tree->dlg = NULL;
        subscription_persistence_remove(sub_tree);
        shutdown_subscriptions(sub_tree->root);
 
@@ -3294,10 +3293,6 @@ static int serialized_pubsub_on_rx_refresh(void *userdata)
        struct sip_subscription_tree *sub_tree = userdata;
        pjsip_dialog *dlg = sub_tree->dlg;
 
-       if (!dlg) {
-               return 0;
-       }
-
        pjsip_dlg_inc_lock(dlg);
        if (!sub_tree->evsub) {
                pjsip_dlg_dec_lock(dlg);