Merged revisions 16742 via svnmerge from
authorKevin P. Fleming <kpfleming@digium.com>
Fri, 31 Mar 2006 18:28:52 +0000 (18:28 +0000)
committerKevin P. Fleming <kpfleming@digium.com>
Fri, 31 Mar 2006 18:28:52 +0000 (18:28 +0000)
https://origsvn.digium.com/svn/asterisk/branches/1.2

........
r16742 | kpfleming | 2006-03-31 12:24:22 -0600 (Fri, 31 Mar 2006) | 2 lines

ensure that hint watchers (subscribers) cannot be added or removed while the dialplan is being modified

........

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@16743 65c4cc65-6c06-0410-ace0-fbb531ad65f3

pbx.c

diff --git a/pbx.c b/pbx.c
index 8221551..2c478ca 100644 (file)
--- a/pbx.c
+++ b/pbx.c
@@ -470,6 +470,12 @@ struct ast_switch *switches = NULL;
 AST_MUTEX_DEFINE_STATIC(switchlock);           /*!< Lock for switches */
 
 static int stateid = 1;
+/* WARNING:
+   When holding this list's lock, do _not_ do anything that will cause conlock
+   to be taken, unless you _already_ hold it. The ast_merge_contexts_and_delete
+   function will take the locks in conlock/hints order, so any other
+   paths that require both locks must also take them in that order.
+*/
 static AST_LIST_HEAD_STATIC(hints, ast_hint);
 struct ast_state_cb *statecbs = NULL;
 
@@ -3508,9 +3514,20 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char
        int length;
        struct ast_state_cb *thiscb, *prevcb;
 
-       /* preserve all watchers for hints associated with this registrar */
        AST_LIST_HEAD_INIT(&store);
+
+       /* it is very important that this function hold the hint list lock _and_ the conlock
+          during its operation; not only do we need to ensure that the list of contexts
+          and extensions does not change, but also that no hint callbacks (watchers) are
+          added or removed during the merge/delete process
+
+          in addition, the locks _must_ be taken in this order, because there are already
+          other code paths that use this order
+       */
+       ast_mutex_lock(&conlock);
        AST_LIST_LOCK(&hints);
+
+       /* preserve all watchers for hints associated with this registrar */
        AST_LIST_TRAVERSE(&hints, hint, list) {
                if (hint->callbacks && !strcmp(registrar, hint->exten->parent->registrar)) {
                        length = strlen(hint->exten->exten) + strlen(hint->exten->parent->name) + 2 + sizeof(*this);
@@ -3526,10 +3543,8 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char
                        AST_LIST_INSERT_HEAD(&store, this, list);
                }
        }
-       AST_LIST_UNLOCK(&hints);
 
        tmp = *extcontexts;
-       ast_mutex_lock(&conlock);
        if (registrar) {
                __ast_context_destroy(NULL,registrar);
                while (tmp) {
@@ -3549,7 +3564,6 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char
                *extcontexts = NULL;
        } else 
                ast_log(LOG_WARNING, "Requested contexts didn't get merged\n");
-       ast_mutex_unlock(&conlock);
 
        /* restore the watchers for hints that can be found; notify those that
           cannot be restored
@@ -3557,7 +3571,6 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char
        while ((this = AST_LIST_REMOVE_HEAD(&store, list))) {
                exten = ast_hint_extension(NULL, this->context, this->exten);
                /* Find the hint in the list of hints */
-               AST_LIST_LOCK(&hints);
                AST_LIST_TRAVERSE(&hints, hint, list) {
                        if (hint->exten == exten)
                                break;
@@ -3580,10 +3593,12 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char
                        hint->callbacks = this->callbacks;
                        hint->laststate = this->laststate;
                }
-               AST_LIST_UNLOCK(&hints);
                free(this);
        }
 
+       AST_LIST_UNLOCK(&hints);
+       ast_mutex_unlock(&conlock);
+
        return; 
 }