[svn-commits] russell: branch russell/messaging r311292 - /team/russell/messaging/main/

[SVN commits to the Digium repositories]

Author: russell
Date: Thu Mar 17 17:10:35 2011
New Revision: 311292

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=311292
Log:
Use a new mechanism for ensuring that message techs are unload safe.

This is now handled in the same way as format attribute interfaces in format.c.

Modified:
    team/russell/messaging/main/message.c

Modified: team/russell/messaging/main/message.c
URL: http://svnview.digium.com/svn/asterisk/team/russell/messaging/main/message.c?view=diff&rev=311292&r1=311291&r2=311292
==============================================================================
--- team/russell/messaging/main/message.c (original)
+++ team/russell/messaging/main/message.c Thu Mar 17 17:10:35 2011
@@ -97,16 +97,16 @@
 				<variable name="MESSAGE_SEND_STATUS">
 					<para>This is the time from dialing a channel until when it is disconnected.</para>
 					<value name="INVALID_PROTOCOL">
-						<para>No handler for the technology part of the URI was found.</para>
+						No handler for the technology part of the URI was found.
 					</value>
 					<value name="INVALID_URI">
-						<para>The protocol handler reported that the URI was not valid.</para>
+						The protocol handler reported that the URI was not valid.
 					</value>
 					<value name="SUCCESS">
-						<para>Successfully passed on to the protocol handler, but delivery has not necessarily been guaranteed.</para>
+						Successfully passed on to the protocol handler, but delivery has not necessarily been guaranteed.
 					</value>
 					<value name="FAILURE">
-						<para>The protocol handler reported that it was unabled to deliver the message for some reason.</para>
+						The protocol handler reported that it was unabled to deliver the message for some reason.
 					</value>
 				</variable>
 			</variablelist>
@@ -128,6 +128,13 @@
 
 struct ast_msg_tech_holder {
 	const struct ast_msg_tech *tech;
+	/*! 
+	 * \brief A rwlock for this object
+	 *
+	 * a read/write lock must be used to protect the wrapper instead
+	 * of the ao2 lock. A rdlock must be held to read tech_holder->tech.
+	 */
+	ast_rwlock_t tech_lock;
 };
 
 static struct ao2_container *msg_techs;
@@ -621,31 +628,34 @@
 
 static int msg_tech_hash(const void *obj, const int flags)
 {
-	const struct ast_msg_tech_holder *tech_holder = obj;
-
-	return ast_str_case_hash(tech_holder->tech->name);
+	struct ast_msg_tech_holder *tech_holder = (struct ast_msg_tech_holder *) obj;
+	int res = 0;
+
+	ast_rwlock_rdlock(&tech_holder->tech_lock);
+	if (tech_holder->tech) {
+		res = ast_str_case_hash(tech_holder->tech->name);
+	}
+	ast_rwlock_unlock(&tech_holder->tech_lock);
+
+	return res;
 }
 
 static int msg_tech_cmp(void *obj, void *arg, int flags)
 {
-	const struct ast_msg_tech_holder *tech_holder = obj;
+	struct ast_msg_tech_holder *tech_holder = obj;
 	const struct ast_msg_tech_holder *tech_holder2 = arg;
-
-	return strcasecmp(tech_holder->tech->name, tech_holder2->tech->name) ?  0 : CMP_MATCH | CMP_STOP;
-}
-
-static int msg_tech_cmp_locked(void *obj, void *arg, int flags)
-{
-	int res;
-	struct ast_msg_tech_holder *tech_holder = obj;
-
-	res = msg_tech_cmp(obj, arg, flags);
-
-	if (res == (CMP_MATCH | CMP_STOP)) {
-		ao2_lock(tech_holder);
-	}
-
-	return res;
+	int res = 1;
+
+	ast_rwlock_rdlock(&tech_holder->tech_lock);
+	/*
+	 * tech_holder2 is a temporary fake tech_holder.
+	 */
+	if (tech_holder->tech) {
+		res = strcasecmp(tech_holder->tech->name, tech_holder2->tech->name) ? 0 : CMP_MATCH | CMP_STOP;
+	}
+	ast_rwlock_unlock(&tech_holder->tech_lock);
+
+	return res ? 0 : CMP_MATCH | CMP_STOP;
 }
 
 /*!
@@ -659,7 +669,7 @@
 	char *tech_name;
 	struct ast_msg_tech_holder *tech_holder = NULL;
 	char *parse;
-	int res;
+	int res = -1;
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(to);
 		AST_APP_ARG(from);
@@ -704,8 +714,7 @@
 			.tech = &tmp_msg_tech,
 		};
 
-		tech_holder = ao2_callback(msg_techs, OBJ_POINTER,
-					msg_tech_cmp_locked, &tmp_tech_holder);
+		tech_holder = ao2_find(msg_techs, &tmp_tech_holder, OBJ_POINTER);
 	}
 
 	if (!tech_holder) {
@@ -720,14 +729,17 @@
 	 * that they could change.
 	 */
 	ao2_lock(msg);
-	res = tech_holder->tech->msg_send(msg, args.to, args.from);
+	ast_rwlock_rdlock(&tech_holder->tech_lock);
+	if (tech_holder->tech) {
+		res = tech_holder->tech->msg_send(msg, args.to, args.from);
+	}
+	ast_rwlock_unlock(&tech_holder->tech_lock);
 	ao2_unlock(msg);
 
 	pbx_builtin_setvar_helper(chan, "MESSAGE_SEND_STATUS", res ? "FAILURE" : "SUCCESS");
 
 exit_cleanup:
 	if (tech_holder) {
-		ao2_unlock(tech_holder);
 		ao2_ref(tech_holder, -1);
 		tech_holder = NULL;
 	}
@@ -755,6 +767,7 @@
 		return -1;
 	}
 
+	ast_rwlock_init(&tech_holder->tech_lock);
 	tech_holder->tech = tech;
 
 	ao2_link(msg_techs, tech_holder);
@@ -781,15 +794,9 @@
 		return -1;
 	}
 
-	/*
-	 * The code that sends messages using a message technology ensures that it
-	 * locks the tech_holder before it returns a reference to it from the
-	 * container.  By just grabbing and releasing the lock here, we ensure that
-	 * we wait until any currently running message send code finishes before
-	 * we proceed with unregistration.
-	 */
-	ao2_lock(tech_holder);
-	ao2_unlock(tech_holder);
+	ast_rwlock_wrlock(&tech_holder->tech_lock);
+	tech_holder->tech = NULL;
+	ast_rwlock_unlock(&tech_holder->tech_lock);
 
 	ao2_ref(tech_holder, -1);
 	tech_holder = NULL;