[asterisk-commits] mmichelson: branch group/CCSS r247835 - in /team/group/CCSS: include/asterisk...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Feb 18 16:29:06 CST 2010
Author: mmichelson
Date: Thu Feb 18 16:29:02 2010
New Revision: 247835
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=247835
Log:
Lock-related changes.
1. Shifted how ast_cc_agent_set_interfaces_chanvar and
ast_set_cc_interfaces_chanvar works a bit by making sure
to not hold the channel lock and monitor list lock at the
same time.
2. Added doxygen notes to public functions which grab locks
so that callers can try to avoid locking order problems.
Modified:
team/group/CCSS/include/asterisk/ccss.h
team/group/CCSS/main/ccss.c
Modified: team/group/CCSS/include/asterisk/ccss.h
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/include/asterisk/ccss.h?view=diff&rev=247835&r1=247834&r2=247835
==============================================================================
--- team/group/CCSS/include/asterisk/ccss.h (original)
+++ team/group/CCSS/include/asterisk/ccss.h Thu Feb 18 16:29:02 2010
@@ -519,6 +519,17 @@
AST_DLLIST_ENTRY(ast_cc_monitor) next;
};
+/*!
+ * \brief Callbacks defined by CC monitors
+ *
+ * \note
+ * Every callback is called with the list of monitors locked. There
+ * are several public API calls that also will try to lock this lock.
+ * These public functions have a note in their doxygen stating so.
+ * As such, pay attention to the lock order you establish in these callbacks
+ * to ensure that you do not violate the lock order when calling
+ * the functions in this file with lock order notices.
+ */
struct ast_cc_monitor_callbacks {
/*!
* \brief Type of monitor the callbacks belong to.
@@ -1287,6 +1298,11 @@
* \since 1.8
* \brief Return the number of outstanding CC requests to a specific device
*
+ * \note
+ * This function will lock the list of monitors stored on every instance of
+ * the CC core. Callers of this function should be aware of this and avoid
+ * any potential lock ordering problems.
+ *
* \param name The name of the monitored device
* \param type The type of the monitored device (e.g. "generic")
* \return The number of CC requests for the monitor
@@ -1348,8 +1364,14 @@
* channel were passed into this function, then the return would be 0 since
* SIP/2000 was not one of the original devices dialed.
*
- * \note This function may be called on a calling channel as well to
+ * \note
+ * This function may be called on a calling channel as well to
* determine if it is part of a CC recall.
+ *
+ * \note
+ * This function will lock the channel as well as the list of monitors
+ * on the channel datastore, though the locks are not held at the same time. Be
+ * sure that you have no potential lock order issues here.
*
* \param chan The channel to check
* \param core_id[out] If this is a valid CC recall, the core_id of the failed call
@@ -1372,6 +1394,11 @@
* within the associated monitor. This function is what one uses to get
* that monitor.
*
+ * \note
+ * This function locks the list of monitors that correspond to the core_id
+ * passed in. Be sure that you have no potential lock order issues when
+ * calling this function.
+ *
* \param core_id The core ID to which this recall corresponds. This likely will
* have been obtained using the ast_cc_is_recall function
* \param device_name Which device to find the monitor for.
@@ -1388,6 +1415,12 @@
* \note
* Implementers of protocol-specific CC agents should call this function after
* calling ast_setup_cc_recall_datastore.
+ *
+ * \note
+ * This function will lock the channel as well as the list of monitors stored
+ * on the channel's CC recall datastore, though neither are held at the same
+ * time. Callers of this function should be aware of potential lock ordering
+ * problems that may arise.
*
* \details
* The CC_INTERFACES channel variable will have the interfaces that should be
@@ -1407,6 +1440,12 @@
* called back for a specific PBX instance. This version of the function is used
* mainly by chan_local, wherein we need to set CC_INTERFACES based on an extension
* and context that appear in the middle of the tree of dialed interfaces
+ *
+ * \note
+ * This function will lock the channel as well as the list of monitors stored
+ * on the channel's CC recall datastore, though neither are held at the same
+ * time. Callers of this function should be aware of potential lock ordering
+ * problems that may arise.
*
* \param chan The channel to set the CC_INTERFACES variable on
* \param extension The name of the extension for which we're setting the variable.
Modified: team/group/CCSS/main/ccss.c
URL: http://svnview.digium.com/svn/asterisk/team/group/CCSS/main/ccss.c?view=diff&rev=247835&r1=247834&r2=247835
==============================================================================
--- team/group/CCSS/main/ccss.c (original)
+++ team/group/CCSS/main/ccss.c Thu Feb 18 16:29:02 2010
@@ -3050,13 +3050,12 @@
ast_str_append(&str, 0, "%s", dialstring_search);
}
-static void set_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_channel *chan)
+static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_str *str)
{
struct extension_monitor_pvt *extension_pvt;
struct extension_child_dialstring *child_dialstring;
struct ast_cc_monitor *monitor_iter = starting_point;
int top_level_id = starting_point->id;
- struct ast_str *var_value = ast_str_create(64);
/* First we need to take all of the is_valid child_dialstrings from
* the extension monitor we found and add them to the CC_INTERFACES
@@ -3065,25 +3064,21 @@
extension_pvt = starting_point->private_data;
AST_LIST_TRAVERSE(&extension_pvt->child_dialstrings, child_dialstring, next) {
if (child_dialstring->is_valid) {
- cc_unique_append(var_value, child_dialstring->original_dialstring);
+ cc_unique_append(str, child_dialstring->original_dialstring);
}
}
/* And now we get the dialstrings from each of the device monitors */
while ((monitor_iter = AST_LIST_NEXT(monitor_iter, next))) {
if (monitor_iter->parent_id == top_level_id) {
- cc_unique_append(var_value, monitor_iter->dialstring);
+ cc_unique_append(str, monitor_iter->dialstring);
}
}
- /* var_value will have an extra '&' tacked onto the end of it, so we need
+ /* str will have an extra '&' tacked onto the end of it, so we need
* to get rid of that.
*/
- ast_str_truncate(var_value, ast_str_strlen(var_value) - 1);
-
- pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(var_value));
- ast_log_dynamic_level(cc_logger_level, "CC_INTERFACES set to %s\n", ast_str_buffer(var_value));
- ast_free(var_value);
+ ast_str_truncate(str, ast_str_strlen(str) - 1);
}
int ast_cc_agent_set_interfaces_chanvar(struct ast_channel *chan)
@@ -3092,10 +3087,16 @@
struct ast_cc_interface_tree *interface_tree;
struct ast_cc_monitor *monitor;
struct cc_recall_ds_data *recall_data;
+ struct ast_str *str = ast_str_create(64);
+
+ if (!str) {
+ return -1;
+ }
ast_channel_lock(chan);
if (!(recall_datastore = ast_channel_datastore_find(chan, &recall_ds_info, NULL))) {
ast_channel_unlock(chan);
+ ast_free(str);
return -1;
}
recall_data = recall_datastore->data;
@@ -3104,10 +3105,14 @@
AST_LIST_LOCK(interface_tree);
monitor = AST_LIST_FIRST(interface_tree);
- set_cc_interfaces_chanvar(monitor, chan);
+ build_cc_interfaces_chanvar(monitor, str);
AST_LIST_UNLOCK(interface_tree);
+ pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str));
+ ast_log_dynamic_level(cc_logger_level, "CC_INTERFACES set to %s\n", ast_str_buffer(str));
+
cc_unref(interface_tree, "Done with interface tree while setting CC_INTERFACES");
+ ast_free(str);
return 0;
}
@@ -3117,10 +3122,16 @@
struct ast_cc_interface_tree *interface_tree;
struct ast_cc_monitor *monitor_iter;
struct cc_recall_ds_data *recall_data;
-
+ struct ast_str *str = ast_str_create(64);
+
+ if (!str) {
+ return -1;
+ }
+
ast_channel_lock(chan);
if (!(recall_datastore = ast_channel_datastore_find(chan, &recall_ds_info, NULL))) {
ast_channel_unlock(chan);
+ ast_free(str);
return -1;
}
recall_data = recall_datastore->data;
@@ -3141,12 +3152,18 @@
*/
AST_LIST_UNLOCK(interface_tree);
cc_unref(interface_tree, "Couldn't find the extension specified");
- return -1;
- }
-
- set_cc_interfaces_chanvar(monitor_iter, chan);
+ ast_free(str);
+ return -1;
+ }
+
+ build_cc_interfaces_chanvar(monitor_iter, str);
AST_LIST_UNLOCK(interface_tree);
+
+ pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str));
+ ast_log_dynamic_level(cc_logger_level, "CC_INTERFACES set to %s\n", ast_str_buffer(str));
+
cc_unref(interface_tree, "Done writing the CC_INTERFACES channel variable");
+ ast_free(str);
return 0;
}
More information about the asterisk-commits
mailing list