[asterisk-commits] jpeeler: trunk r302270 - in /trunk: ./ main/pbx.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Jan 18 14:41:07 CST 2011
Author: jpeeler
Date: Tue Jan 18 14:40:59 2011
New Revision: 302270
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=302270
Log:
Merged revisions 302266 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
................
r302266 | jpeeler | 2011-01-18 14:19:57 -0600 (Tue, 18 Jan 2011) | 34 lines
Merged revisions 302265 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2
........
r302265 | jpeeler | 2011-01-18 14:13:52 -0600 (Tue, 18 Jan 2011) | 27 lines
Convert device state callbacks to ao2 objects to fix a deadlock in chan_sip.
Lock scenario presented here:
Thread 1
holds ast_rdlock_contexts &conlock
holds handle_statechange hints
holds handle_statechange hint
waiting for cb_extensionstate
Locked Here: chan_sip.c line 7428 (find_call)
Thread 2
holds handle_request_do &netlock
holds find_call sip_pvt_ptr
waiting for ast_rdlock_contexts &conlock
Locked Here: pbx.c line 9911 (ast_rdlock_contexts)
Chan_sip has an established locking order of locking the sip_pvt and then
getting the context lock. So the as stated by the summary, the operations in
thread 2 have been modified to no longer require the context lock.
(closes issue #18310)
Reported by: one47
Patches:
statecbs_ao2.mk2.patch uploaded by one47 (license 23),
modified by me
Review: https://reviewboard.asterisk.org/r/1072/
........
................
Modified:
trunk/ (props changed)
trunk/main/pbx.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: trunk/main/pbx.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/pbx.c?view=diff&rev=302270&r1=302269&r2=302270
==============================================================================
--- trunk/main/pbx.c (original)
+++ trunk/main/pbx.c Tue Jan 18 14:40:59 2011
@@ -933,7 +933,7 @@
struct ast_hint {
struct ast_exten *exten; /*!< Extension */
int laststate; /*!< Last known state */
- AST_LIST_HEAD_NOLOCK(, ast_state_cb) callbacks; /*!< Callback list for this extension */
+ struct ao2_container *callbacks; /*!< Callback container for this extension */
};
@@ -1282,8 +1282,7 @@
*/
static struct ao2_container *hints;
-/* XXX TODO Convert this to an astobj2 container, too. */
-static AST_LIST_HEAD_NOLOCK_STATIC(statecbs, ast_state_cb);
+static struct ao2_container *statecbs;
#ifdef CONTEXT_DEBUG
@@ -4344,9 +4343,9 @@
struct ast_hint *hint;
struct ast_hintdevice *device, *cmpdevice;
struct statechange *sc = datap;
- struct ast_state_cb *cblist;
int state;
struct ao2_iterator *iterator;
+ struct ao2_iterator cb_iter;
if (ao2_container_count(hintdevices) == 0) {
return 0;
@@ -4363,6 +4362,7 @@
cmpdevice,
"find devices in container");
while (iterator && (device = ao2_iterator_next(iterator))) {
+ struct ast_state_cb *state_cb;
if (!device->hint) {
ao2_t_ref(device, -1, "no hint linked to device");
continue;
@@ -4378,7 +4378,6 @@
/* Device state changed since last check - notify the watchers */
- ast_rdlock_contexts();
ao2_lock(hints);
ao2_lock(hint);
@@ -4387,25 +4386,27 @@
ao2_unlock(hint);
ao2_t_ref(device, -1, "linked hint from device has no exten");
ao2_unlock(hints);
- ast_unlock_contexts();
continue;
}
/* For general callbacks */
- AST_LIST_TRAVERSE(&statecbs, cblist, entry) {
- cblist->callback(hint->exten->parent->name, hint->exten->exten, state, cblist->data);
- }
+ cb_iter = ao2_iterator_init(statecbs, 0);
+ for (state_cb = ao2_iterator_next(&cb_iter); state_cb; ao2_ref(state_cb, -1), state_cb = ao2_iterator_next(&cb_iter)) {
+ state_cb->callback(hint->exten->parent->name, hint->exten->exten, state, state_cb->data);
+ }
+ ao2_iterator_destroy(&cb_iter);
/* For extension callbacks */
- AST_LIST_TRAVERSE(&hint->callbacks, cblist, entry) {
- cblist->callback(hint->exten->parent->name, hint->exten->exten, state, cblist->data);
- }
+ cb_iter = ao2_iterator_init(hint->callbacks, 0);
+ for (state_cb = ao2_iterator_next(&cb_iter); state_cb; ao2_ref(state_cb, -1), state_cb = ao2_iterator_next(&cb_iter)) {
+ state_cb->callback(hint->exten->parent->name, hint->exten->exten, state, state_cb->data);
+ }
+ ao2_iterator_destroy(&cb_iter);
hint->laststate = state; /* record we saw the change */
ao2_unlock(hint);
ao2_t_ref(device, -1, "finished callbacks for device");
ao2_unlock(hints);
- ast_unlock_contexts();
}
if (iterator) {
ao2_iterator_destroy(iterator);
@@ -4421,31 +4422,32 @@
ast_state_cb_type callback, void *data)
{
struct ast_hint *hint;
- struct ast_state_cb *cblist;
+ struct ast_state_cb *state_cb;
struct ast_exten *e;
/* If there's no context and extension: add callback to statecbs list */
if (!context && !exten) {
ao2_lock(hints);
- AST_LIST_TRAVERSE(&statecbs, cblist, entry) {
- if (cblist->callback == callback) {
- cblist->data = data;
- ao2_unlock(hints);
- return 0;
- }
+ state_cb = ao2_find(statecbs, callback, 0);
+ if (state_cb) {
+ state_cb->data = data;
+ ao2_ref(state_cb, -1);
+ ao2_unlock(hints);
+ return 0;
}
/* Now insert the callback */
- if (!(cblist = ast_calloc(1, sizeof(*cblist)))) {
+ if (!(state_cb = ao2_alloc(sizeof(*state_cb), NULL))) {
ao2_unlock(hints);
return -1;
}
- cblist->id = 0;
- cblist->callback = callback;
- cblist->data = data;
-
- AST_LIST_INSERT_HEAD(&statecbs, cblist, entry);
+ state_cb->id = 0;
+ state_cb->callback = callback;
+ state_cb->data = data;
+
+ ao2_link(statecbs, state_cb);
+ ao2_ref(state_cb, -1);
ao2_unlock(hints);
return 0;
@@ -4482,35 +4484,35 @@
}
/* Now insert the callback in the callback list */
- if (!(cblist = ast_calloc(1, sizeof(*cblist)))) {
+ if (!(state_cb = ao2_alloc(sizeof(*state_cb), NULL))) {
ao2_ref(hint, -1);
return -1;
}
- cblist->id = stateid++; /* Unique ID for this callback */
- cblist->callback = callback; /* Pointer to callback routine */
- cblist->data = data; /* Data for the callback */
+ state_cb->id = stateid++; /* Unique ID for this callback */
+ state_cb->callback = callback; /* Pointer to callback routine */
+ state_cb->data = data; /* Data for the callback */
ao2_lock(hint);
- AST_LIST_INSERT_HEAD(&hint->callbacks, cblist, entry);
+ ao2_link(hint->callbacks, state_cb);
+ ao2_ref(state_cb, -1);
ao2_unlock(hint);
ao2_ref(hint, -1);
- return cblist->id;
+ return state_cb->id;
}
/*! \brief Find Hint by callback id */
static int find_hint_by_cb_id(void *obj, void *arg, int flags)
{
+ struct ast_state_cb *state_cb;
const struct ast_hint *hint = obj;
int *id = arg;
- struct ast_state_cb *cb;
-
- AST_LIST_TRAVERSE(&hint->callbacks, cb, entry) {
- if (cb->id == *id) {
- return CMP_MATCH | CMP_STOP;
- }
+
+ if ((state_cb = ao2_find(hint->callbacks, id, 0))) {
+ ao2_ref(state_cb, -1);
+ return CMP_MATCH | CMP_STOP;
}
return 0;
@@ -4528,13 +4530,11 @@
if (!id) { /* id == 0 is a callback without extension */
ao2_lock(hints);
- AST_LIST_TRAVERSE_SAFE_BEGIN(&statecbs, p_cur, entry) {
- if (p_cur->callback == callback) {
- AST_LIST_REMOVE_CURRENT(entry);
- break;
- }
- }
- AST_LIST_TRAVERSE_SAFE_END;
+ p_cur = ao2_find(statecbs, callback, OBJ_UNLINK);
+ if (p_cur) {
+ ret = 0;
+ ao2_ref(p_cur, -1);
+ }
ao2_unlock(hints);
} else { /* callback with extension, find the callback based on ID */
struct ast_hint *hint;
@@ -4543,27 +4543,27 @@
if (hint) {
ao2_lock(hint);
- AST_LIST_TRAVERSE_SAFE_BEGIN(&hint->callbacks, p_cur, entry) {
- if (p_cur->id == id) {
- AST_LIST_REMOVE_CURRENT(entry);
- ret = 0;
- break;
- }
- }
- AST_LIST_TRAVERSE_SAFE_END;
-
+ p_cur = ao2_find(hint->callbacks, &id, OBJ_UNLINK);
+ if (p_cur) {
+ ret = 0;
+ ao2_ref(p_cur, -1);
+ }
ao2_unlock(hint);
ao2_ref(hint, -1);
}
}
- if (p_cur) {
- ast_free(p_cur);
- }
-
return ret;
}
+
+static int hint_id_cmp(void *obj, void *arg, int flags)
+{
+ const struct ast_state_cb *cb = obj;
+ int *id = arg;
+
+ return (cb->id == *id) ? CMP_MATCH | CMP_STOP : 0;
+}
/*! \brief Add hint to hint list, check initial extension state */
static int ast_add_hint(struct ast_exten *e)
@@ -4585,6 +4585,9 @@
ast_debug(2, "HINTS: Adding hint %s: %s\n", ast_get_extension_name(e), ast_get_extension_app(e));
if (!(hint = ao2_alloc(sizeof(*hint), NULL))) {
+ return -1;
+ }
+ if (!(hint->callbacks = ao2_container_alloc(1, NULL, hint_id_cmp))) {
return -1;
}
@@ -4635,7 +4638,7 @@
{
/* Cleanup the Notifys if hint is removed */
struct ast_hint *hint;
- struct ast_state_cb *cblist;
+ struct ast_state_cb *state_cb;
if (!e) {
return -1;
@@ -4648,15 +4651,16 @@
}
ao2_lock(hint);
- while ((cblist = AST_LIST_REMOVE_HEAD(&hint->callbacks, entry))) {
+ while ((state_cb = ao2_callback(hint->callbacks, OBJ_UNLINK, NULL, NULL))) {
/* Notify with -1 and remove all callbacks */
- cblist->callback(hint->exten->parent->name, hint->exten->exten,
- AST_EXTENSION_DEACTIVATED, cblist->data);
- ast_free(cblist);
+ state_cb->callback(hint->exten->parent->name, hint->exten->exten,
+ AST_EXTENSION_DEACTIVATED, state_cb->data);
+ ao2_ref(state_cb, -1);
}
remove_hintdevice(hint);
hint->exten = NULL;
ao2_unlink(hints, hint);
+ ao2_ref(hint->callbacks, -1);
ao2_unlock(hint);
ao2_ref(hint, -1);
@@ -5876,7 +5880,6 @@
struct ast_hint *hint;
int num = 0;
int watchers;
- struct ast_state_cb *watcher;
struct ao2_iterator i;
switch (cmd) {
@@ -5900,10 +5903,7 @@
i = ao2_iterator_init(hints, 0);
for (hint = ao2_iterator_next(&i); hint; ao2_ref(hint, -1), hint = ao2_iterator_next(&i)) {
- watchers = 0;
- AST_LIST_TRAVERSE(&hint->callbacks, watcher, entry) {
- watchers++;
- }
+ watchers = ao2_container_count(hint->callbacks);
ast_cli(a->fd, " %20s@%-20.20s: %-20.20s State:%-15.15s Watchers %2d\n",
ast_get_extension_name(hint->exten),
ast_get_context_name(ast_get_extension_context(hint->exten)),
@@ -5953,7 +5953,6 @@
struct ast_hint *hint;
int watchers;
int num = 0, extenlen;
- struct ast_state_cb *watcher;
struct ao2_iterator i;
switch (cmd) {
@@ -5980,10 +5979,7 @@
for (hint = ao2_iterator_next(&i); hint; ao2_unlock(hint), ao2_ref(hint, -1), hint = ao2_iterator_next(&i)) {
ao2_lock(hint);
if (!strncasecmp(hint->exten ? ast_get_extension_name(hint->exten) : "", a->argv[3], extenlen)) {
- watchers = 0;
- AST_LIST_TRAVERSE(&hint->callbacks, watcher, entry) {
- watchers++;
- }
+ watchers = ao2_container_count(hint->callbacks);
ast_cli(a->fd, " %20s@%-20.20s: %-20.20s State:%-15.15s Watchers %2d\n",
ast_get_extension_name(hint->exten),
ast_get_context_name(ast_get_extension_context(hint->exten)),
@@ -7211,7 +7207,8 @@
/* preserve all watchers for hints */
i = ao2_iterator_init(hints, AO2_ITERATOR_DONTLOCK);
for (hint = ao2_iterator_next(&i); hint; ao2_ref(hint, -1), hint = ao2_iterator_next(&i)) {
- if (!AST_LIST_EMPTY(&hint->callbacks)) {
+ if (ao2_container_count(hint->callbacks)) {
+
length = strlen(hint->exten->exten) + strlen(hint->exten->parent->name) + 2 + sizeof(*this);
if (!(this = ast_calloc(1, length))) {
continue;
@@ -7222,8 +7219,12 @@
ao2_unlock(hint);
continue;
}
- /* this removes all the callbacks from the hint into this. */
- AST_LIST_APPEND_LIST(&this->callbacks, &hint->callbacks, entry);
+ /* this removes all the callbacks from the hint into 'this'. */
+ while ((thiscb = ao2_callback(hint->callbacks, OBJ_UNLINK, NULL, NULL))) {
+ AST_LIST_INSERT_TAIL(&this->callbacks, thiscb, entry);
+ /* We intentionally do not unref thiscb to account for the non-ao2 reference in this->callbacks */
+ }
+
this->laststate = hint->laststate;
this->context = this->data;
strcpy(this->data, hint->exten->parent->name);
@@ -7265,11 +7266,14 @@
/* this hint has been removed, notify the watchers */
while ((thiscb = AST_LIST_REMOVE_HEAD(&this->callbacks, entry))) {
thiscb->callback(this->context, this->exten, AST_EXTENSION_REMOVED, thiscb->data);
- ast_free(thiscb);
+ ao2_ref(thiscb, -1); /* Ref that we added when putting into this->callbacks */
}
} else {
ao2_lock(hint);
- AST_LIST_APPEND_LIST(&hint->callbacks, &this->callbacks, entry);
+ while ((thiscb = AST_LIST_REMOVE_HEAD(&this->callbacks, entry))) {
+ ao2_link(hint->callbacks, thiscb);
+ ao2_ref(thiscb, -1); /* Ref that we added when putting into this->callbacks */
+ }
hint->laststate = this->laststate;
ao2_unlock(hint);
}
@@ -9948,7 +9952,6 @@
struct ast_data *data_hint;
struct ast_hint *hint;
int watchers;
- struct ast_state_cb *watcher;
struct ao2_iterator i;
if (ao2_container_count(hints) == 0) {
@@ -9957,10 +9960,7 @@
i = ao2_iterator_init(hints, 0);
for (hint = ao2_iterator_next(&i); hint; ao2_ref(hint, -1), hint = ao2_iterator_next(&i)) {
- watchers = 0;
- AST_LIST_TRAVERSE(&hint->callbacks, watcher, entry) {
- watchers++;
- }
+ watchers = ao2_container_count(hint->callbacks);
data_hint = ast_data_add_node(data_root, "hint");
if (!data_hint) {
continue;
@@ -10367,10 +10367,19 @@
return (hint->exten == exten) ? CMP_MATCH | CMP_STOP : 0;
}
+static int statecbs_cmp(void *obj, void *arg, int flags)
+{
+ const struct ast_state_cb *state_cb = obj;
+ const struct ast_state_cb *callback = arg;
+
+ return (state_cb == callback) ? CMP_MATCH | CMP_STOP : 0;
+}
+
int ast_pbx_init(void)
{
hints = ao2_container_alloc(HASH_EXTENHINT_SIZE, hint_hash, hint_cmp);
hintdevices = ao2_container_alloc(HASH_EXTENHINT_SIZE, hintdevice_hash_cb, hintdevice_cmp_multiple);
-
- return (hints && hintdevices) ? 0 : -1;
-}
+ statecbs = ao2_container_alloc(HASH_EXTENHINT_SIZE, NULL, statecbs_cmp);
+
+ return (hints && hintdevices && statecbs) ? 0 : -1;
+}
More information about the asterisk-commits
mailing list