[asterisk-commits] mmichelson: branch mmichelson/pub_sub r385907 - /team/mmichelson/pub_sub/res/

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Tue Apr 16 13:55:21 2013
New Revision: 385907

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=385907
Log:
Some final comments for the road.


Modified:
    team/mmichelson/pub_sub/res/res_sip_pubsub.c

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=385907&r1=385906&r2=385907
==============================================================================
--- team/mmichelson/pub_sub/res/res_sip_pubsub.c (original)
+++ team/mmichelson/pub_sub/res/res_sip_pubsub.c Tue Apr 16 13:55:21 2013
@@ -42,13 +42,23 @@
 	.on_rx_request = sub_on_rx_request,
 };
 
+/*!
+ * \brief Structure representing a SIP subscription
+ */
 struct ast_sip_subscription {
+	/*! Subscription datastores set up by handlers */
 	struct ao2_container *datastores;
+	/*! The endpoint with which the subscription is communicating */
 	struct ast_sip_endpoint *endpoint;
+	/*! Serializer on which to place operations for this subscription */
 	struct ast_taskprocessor *serializer;
+	/*! The handler for this subscription */
 	const struct ast_sip_subscription_handler *handler;
+	/*! The role for this subscription */
 	enum ast_sip_subscription_role role;
+	/*! The underlying PJSIP event subscription structure */
 	pjsip_evsub *evsub;
+	/*! The underlying PJSIP dialog */
 	pjsip_dialog *dlg;
 };
 
@@ -82,6 +92,15 @@
 {
 	pjsip_dialog *dlg = user_data;
 
+	/* 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.
+	 */
 	ast_sip_dialog_remove_serializer(dlg);
 	pjsip_dlg_dec_session(dlg, &sub_module);
 
@@ -96,6 +115,10 @@
 
 	ao2_cleanup(sub->datastores);
 	ao2_cleanup(sub->endpoint);
+
+	/* This is pushed into the threadpool since there is no guarantee
+	 * what thread this destructor will get called from
+	 */
 	if (sub->dlg) {
 		ast_sip_push_task_synchronous(NULL, remove_serializer, sub->dlg);
 	}
@@ -191,6 +214,9 @@
 		return NULL;
 	}
 	sub->evsub = allocate_evsub(handler->event_name, role, endpoint, rdata, dlg);
+	/* We keep a reference to the dialog until our subscription is destroyed. See
+	 * the subscription_destructor for more details
+	 */
 	pjsip_dlg_inc_session(dlg, &sub_module);
 	sub->dlg = dlg;
 	ast_sip_dialog_set_serializer(dlg, sub->serializer);