[asterisk-commits] dvossel: branch dvossel/masq_locking_order_trunk r221463 - in /team/dvossel/m...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed Sep 30 16:26:29 CDT 2009
Author: dvossel
Date: Wed Sep 30 16:26:24 2009
New Revision: 221463
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=221463
Log:
ast_do_masqurade now assumes no channels are locked. This allows
the function to first unlink both of the channels, lock them both,
and re-link them back into the table at the end of the function. This
way it doesn't matter if the names of the channels change or not.
Modified:
team/dvossel/masq_locking_order_trunk/channels/chan_misdn.c
team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h
team/dvossel/masq_locking_order_trunk/main/channel.c
Modified: team/dvossel/masq_locking_order_trunk/channels/chan_misdn.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/channels/chan_misdn.c?view=diff&rev=221463&r1=221462&r2=221463
==============================================================================
--- team/dvossel/masq_locking_order_trunk/channels/chan_misdn.c (original)
+++ team/dvossel/masq_locking_order_trunk/channels/chan_misdn.c Wed Sep 30 16:26:24 2009
@@ -7758,9 +7758,7 @@
snprintf(newname, sizeof(newname), "%s/%d-", misdn_type, chan_offset + c);
if (strncmp(tmp->name, newname, strlen(newname))) {
snprintf(newname, sizeof(newname), "%s/%d-u%d", misdn_type, chan_offset + c, glob_channel++);
- ast_channel_lock(tmp);
ast_change_name(tmp, newname);
- ast_channel_unlock(tmp);
chan_misdn_log(3, port, " --> updating channel name to [%s]\n", tmp->name);
}
}
Modified: team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h?view=diff&rev=221463&r1=221462&r2=221463
==============================================================================
--- team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h (original)
+++ team/dvossel/masq_locking_order_trunk/include/asterisk/channel.h Wed Sep 30 16:26:24 2009
@@ -1054,7 +1054,7 @@
/*!
* \brief Change channel name
*
- * \pre The channel must be locked before calling this function.
+ * \pre Absolutely all channels _MUST_ be unlocked before calling this function.
*
* \param chan the channel to change the name of
* \param newname the name to change to
@@ -1904,6 +1904,7 @@
/*!
* \brief Start masquerading a channel
+ * \note absolutely _NO_ channel locks should be held before calling this function.
* \details
* XXX This is a seriously whacked out operation. We're essentially putting the guts of
* the clone channel into the original channel. Start by killing off the original
Modified: team/dvossel/masq_locking_order_trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/masq_locking_order_trunk/main/channel.c?view=diff&rev=221463&r1=221462&r2=221463
==============================================================================
--- team/dvossel/masq_locking_order_trunk/main/channel.c (original)
+++ team/dvossel/masq_locking_order_trunk/main/channel.c Wed Sep 30 16:26:24 2009
@@ -4813,15 +4813,27 @@
return res;
}
+/*! \brief this function simply changes the name of the channel and issues a manager_event
+ * with out unlinking and linking the channel from the ao2_container. This should
+ * only be used when the channel has already been unlinked from the ao2_container.
+ */
+static void __ast_change_name_nolink(struct ast_channel *chan, const char *newname)
+{
+ manager_event(EVENT_FLAG_CALL, "Rename", "Channel: %s\r\nNewname: %s\r\nUniqueid: %s\r\n", chan->name, newname, chan->uniqueid);
+ ast_string_field_set(chan, name, newname);
+}
+
+/*! \note this function must _NEVER_ ever ever be used when any channels
+ * are locked regardless if it is the channel who's name is being changed
+ * or not because it invalidates our channel container locking order...
+ * lock container first, then the channels, never the other way around. */
void ast_change_name(struct ast_channel *chan, const char *newname)
{
/* We must re-link, as the hash value will change here. */
ao2_unlink(channels, chan);
-
- manager_event(EVENT_FLAG_CALL, "Rename", "Channel: %s\r\nNewname: %s\r\nUniqueid: %s\r\n", chan->name, newname, chan->uniqueid);
-
- ast_string_field_set(chan, name, newname);
-
+ ast_channel_lock(chan);
+ __ast_change_name_nolink(chan, newname);
+ ast_channel_unlock(chan);
ao2_link(channels, chan);
}
@@ -5101,9 +5113,12 @@
* original channel's backend. While the features are nice, which is the
* reason we're keeping it, it's still awesomely weird. XXX */
- /* We need the clone's lock, too */
- ast_channel_lock(clonechan);
-
+ ao2_unlink(channels, original);
+ ao2_unlink(channels, clonechan);
+
+ /* Now that we have unlinked from the channels container, We need both
+ * the original and clone channels locked */
+ ast_channel_lock_both(original, clonechan);
ast_debug(4, "Actually Masquerading %s(%d) into the structure of %s(%d)\n",
clonechan->name, clonechan->_state, original->name, original->_state);
@@ -5127,10 +5142,10 @@
snprintf(masqn, sizeof(masqn), "%s<MASQ>", newn);
/* Mangle the name of the clone channel */
- ast_change_name(clonechan, masqn);
+ __ast_change_name_nolink(clonechan, masqn);
/* Copy the name from the clone channel */
- ast_change_name(original, newn);
+ __ast_change_name_nolink(original, newn);
/* share linked id's */
ast_channel_set_linkgroup(original, clonechan);
@@ -5204,24 +5219,20 @@
original->_state = clonechan->_state;
clonechan->_state = origstate;
- if (clonechan->tech->fixup){
- res = clonechan->tech->fixup(original, clonechan);
- if (res)
- ast_log(LOG_WARNING, "Fixup failed on channel %s, strange things may happen.\n", clonechan->name);
+ if (clonechan->tech->fixup && clonechan->tech->fixup(original, clonechan)) {
+ ast_log(LOG_WARNING, "Fixup failed on channel %s, strange things may happen.\n", clonechan->name);
}
/* Start by disconnecting the original's physical side */
- if (clonechan->tech->hangup)
- res = clonechan->tech->hangup(clonechan);
- if (res) {
+ if (clonechan->tech->hangup && clonechan->tech->hangup(clonechan)) {
ast_log(LOG_WARNING, "Hangup failed! Strange things may happen!\n");
- ast_channel_unlock(clonechan);
- return -1;
+ res = -1;
+ goto done;
}
/* Mangle the name of the clone channel */
- snprintf(zombn, sizeof(zombn), "%s<ZOMBIE>", orig);
- ast_change_name(clonechan, zombn);
+ snprintf(zombn, sizeof(zombn), "%s<ZOMBIE>", orig); /* quick, hide the brains! */
+ __ast_change_name_nolink(clonechan, zombn);
/* Update the type. */
t_pvt = original->monitor;
@@ -5315,12 +5326,11 @@
/* Okay. Last thing is to let the channel driver know about all this mess, so he
can fix up everything as best as possible */
if (original->tech->fixup) {
- res = original->tech->fixup(clonechan, original);
- if (res) {
+ if (original->tech->fixup(clonechan, original)) {
ast_log(LOG_WARNING, "Channel for type '%s' could not fixup channel %s\n",
original->tech->type, original->name);
- ast_channel_unlock(clonechan);
- return -1;
+ res = -1;
+ goto done;
}
} else
ast_log(LOG_WARNING, "Channel type '%s' does not have a fixup routine (for %s)! Bad things may happen.\n",
@@ -5354,18 +5364,29 @@
clonechan->hangupcause,
ast_cause2str(clonechan->hangupcause)
);
- clonechan = ast_channel_release(clonechan);
+ ast_channel_release(clonechan);
+ clonechan = NULL;
} else {
ast_debug(1, "Released clone lock on '%s'\n", clonechan->name);
ast_set_flag(clonechan, AST_FLAG_ZOMBIE);
ast_queue_frame(clonechan, &ast_null_frame);
- ast_channel_unlock(clonechan);
}
/* Signal any blocker */
if (ast_test_flag(original, AST_FLAG_BLOCKING))
pthread_kill(original->blocker, SIGURG);
ast_debug(1, "Done Masquerading %s (%d)\n", original->name, original->_state);
+
+
+done:
+ /* it is possible for the clone channel to disappear during this */
+ if (clonechan) {
+ ast_channel_unlock(clonechan);
+ ao2_link(channels, clonechan);
+ }
+ ast_channel_unlock(original);
+ ao2_link(channels, original);
+
return 0;
}
More information about the asterisk-commits
mailing list