[asterisk-commits] mmichelson: branch mmichelson/pub_sub r385905 - in /team/mmichelson/pub_sub: ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Apr 16 13:29:28 CDT 2013


Author: mmichelson
Date: Tue Apr 16 13:29:25 2013
New Revision: 385905

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=385905
Log:
Fix a crash seen during solicited MWI testing.

The issue stemmed from the fact that the state change to
PJSIP_EVSUB_STATE_TERMINATED does not necessarily correspond with
the destruction of the underlying dialog. However, we use this
state change as our means of facilitating subscription destruction,
because it's the best way we have of doing so. Unfortunately, since
the dialog still exists, it means that the distributor will attempt
to use the serializer saved on the dialog, which unfortunately
has become invalid. Attempting to use the invalid serializer results
in a crash.

The fix put in place is to add a function that allows for an application
to remove a serializer from a dialog. This is useful for a case like this
where the serializer may disappear before the dialog has actually been
destroyed. This way, if any further requests/responses arrive on the
dialog, the distributor will not attempt to use an invalid serializer.
Instead, there will be no serializer used at all, which is fine since
our application that cares about serialization is no longer involved.

With this change, I realize that some explanatory comments are necessary
in some of the code, so I will add those and then put this up for
review.


Modified:
    team/mmichelson/pub_sub/include/asterisk/res_sip.h
    team/mmichelson/pub_sub/res/res_sip.exports.in
    team/mmichelson/pub_sub/res/res_sip/sip_distributor.c
    team/mmichelson/pub_sub/res/res_sip_pubsub.c

Modified: team/mmichelson/pub_sub/include/asterisk/res_sip.h
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/pub_sub/include/asterisk/res_sip.h?view=diff&rev=385905&r1=385904&r2=385905
==============================================================================
--- team/mmichelson/pub_sub/include/asterisk/res_sip.h (original)
+++ team/mmichelson/pub_sub/include/asterisk/res_sip.h Tue Apr 16 13:29:25 2013
@@ -792,6 +792,17 @@
 void ast_sip_dialog_set_serializer(pjsip_dialog *dlg, struct ast_taskprocessor *serializer);
 
 /*!
+ * \brief Remove a serializer on a SIP dialog
+ *
+ * This is useful for a case where the user of the serializer dies before the SIP dialog
+ * does. This way, any further requests/responses that arrive on the dialog do not try
+ * to use the serializer any more
+ *
+ * \param Dialog from which to remove the serializer
+ */
+void ast_sip_dialog_remove_serializer(pjsip_dialog *dlg);
+
+/*!
  * \brief Set an endpoint on a SIP dialog so in-dialog requests do not undergo endpoint lookup.
  *
  * \param dlg The SIP dialog itself

Modified: team/mmichelson/pub_sub/res/res_sip.exports.in
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/pub_sub/res/res_sip.exports.in?view=diff&rev=385905&r1=385904&r2=385905
==============================================================================
--- team/mmichelson/pub_sub/res/res_sip.exports.in (original)
+++ team/mmichelson/pub_sub/res/res_sip.exports.in Tue Apr 16 13:29:25 2013
@@ -42,6 +42,7 @@
 		LINKER_SYMBOL_PREFIXast_pjsip_rdata_get_endpoint;
 		LINKER_SYMBOL_PREFIXast_sip_thread_is_servant;
 		LINKER_SYMBOL_PREFIXast_sip_dialog_set_serializer;
+		LINKER_SYMBOL_PREFIXast_sip_dialog_remove_serializer;
 		LINKER_SYMBOL_PREFIXast_sip_dialog_set_endpoint;
 		LINKER_SYMBOL_PREFIXast_sip_dialog_get_endpoint;
 		LINKER_SYMBOL_PREFIXast_sip_retrieve_auths;

Modified: team/mmichelson/pub_sub/res/res_sip/sip_distributor.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/pub_sub/res/res_sip/sip_distributor.c?view=diff&rev=385905&r1=385904&r2=385905
==============================================================================
--- team/mmichelson/pub_sub/res/res_sip/sip_distributor.c (original)
+++ team/mmichelson/pub_sub/res/res_sip/sip_distributor.c Tue Apr 16 13:29:25 2013
@@ -61,6 +61,17 @@
 	dist->serializer = serializer;
 }
 
+void ast_sip_dialog_remove_serializer(pjsip_dialog *dlg)
+{
+	struct distributor_dialog_data *dist;
+	SCOPED_LOCK(lock, dlg, pjsip_dlg_inc_lock, pjsip_dlg_dec_lock);
+
+	dist = pjsip_dlg_get_mod_data(dlg, distributor_mod.id);
+	if (dist) {
+		dist->serializer = NULL;
+	}
+}
+
 void ast_sip_dialog_set_endpoint(pjsip_dialog *dlg, struct ast_sip_endpoint *endpoint)
 {
 	struct distributor_dialog_data *dist = pjsip_dlg_get_mod_data(dlg, distributor_mod.id);
@@ -72,7 +83,10 @@
 
 struct ast_sip_endpoint *ast_sip_dialog_get_endpoint(pjsip_dialog *dlg)
 {
-	struct distributor_dialog_data *dist = pjsip_dlg_get_mod_data(dlg, distributor_mod.id);
+	struct distributor_dialog_data *dist;
+	SCOPED_LOCK(lock, dlg, pjsip_dlg_inc_lock, pjsip_dlg_dec_lock);
+
+	dist = pjsip_dlg_get_mod_data(dlg, distributor_mod.id);
 	if (!dist || !dist->endpoint) {
 		return NULL;
 	}

Modified: team/mmichelson/pub_sub/res/res_sip_pubsub.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/pub_sub/res/res_sip_pubsub.c?view=diff&rev=385905&r1=385904&r2=385905
==============================================================================
--- team/mmichelson/pub_sub/res/res_sip_pubsub.c (original)
+++ team/mmichelson/pub_sub/res/res_sip_pubsub.c Tue Apr 16 13:29:25 2013
@@ -49,6 +49,7 @@
 	const struct ast_sip_subscription_handler *handler;
 	enum ast_sip_subscription_role role;
 	pjsip_evsub *evsub;
+	pjsip_dialog *dlg;
 };
 
 #define DATASTORE_BUCKETS 53
@@ -77,12 +78,27 @@
 	return strcmp(datastore1->uid, uid2) ? 0 : CMP_MATCH | CMP_STOP;
 }
 
+static int remove_serializer(void *user_data)
+{
+	pjsip_dialog *dlg = user_data;
+
+	ast_sip_dialog_remove_serializer(dlg);
+	pjsip_dlg_dec_session(dlg, &sub_module);
+
+	return 0;
+}
+
 static void subscription_destructor(void *obj)
 {
 	struct ast_sip_subscription *sub = obj;
+
+	ast_debug(3, "Destroying SIP subscription\n");
 
 	ao2_cleanup(sub->datastores);
 	ao2_cleanup(sub->endpoint);
+	if (sub->dlg) {
+		ast_sip_push_task_synchronous(NULL, remove_serializer, sub->dlg);
+	}
 	ast_taskprocessor_unreference(sub->serializer);
 }
 
@@ -175,6 +191,8 @@
 		return NULL;
 	}
 	sub->evsub = allocate_evsub(handler->event_name, role, endpoint, rdata, dlg);
+	pjsip_dlg_inc_session(dlg, &sub_module);
+	sub->dlg = dlg;
 	ast_sip_dialog_set_serializer(dlg, sub->serializer);
 	pjsip_evsub_set_mod_data(sub->evsub, sub_module.id, sub);
 	ao2_ref(endpoint, +1);




More information about the asterisk-commits mailing list