kenmirrors/asterisk
1
0
Fork 0
mirror of https://github.com/asterisk/asterisk.git synced 2025-11-09 11:28:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

Allow AMI action callback to be reentrant.

Browse Source 
Fix AMI module reload deadlock regression from ASTERISK-18479 when it
tried to fix the race between calling an AMI action callback and
unregistering that action.  Refixes ASTERISK-13784 broken by
ASTERISK-17785 change.

Locking the ao2 object guaranteed that there were no active callbacks that
mattered when ast_manager_unregister() was called.  Unfortunately, this
causes the deadlock situation.  The patch stops locking the ao2 object to
allow multiple threads to invoke the callback re-entrantly.  There is no
way to guarantee a module unload will not crash because of an active
callback.  The code attempts to minimize the chance with the registered
flag and the maximum 5 second delay before ast_manager_unregister()
returns.

The trunk version of the patch changes the API to fix the race condition
correctly to prevent the module code from unloading from memory while an
action callback is active.

* Don't hold the lock while calling the AMI action callback.

(closes issue ASTERISK-19487)
Reported by: Philippe Lindheimer

Review: https://reviewboard.asterisk.org/r/1818/
Review: https://reviewboard.asterisk.org/r/1820/


git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@359979 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Richard Mudgett
2012-03-20 17:21:16 +00:00
parent 698cd736b6
commit 8dc6345d0f
2 changed files with 29 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/asterisk/manager.h
View File

@@ -160,6 +160,8 @@ struct manager_action {
* function and unregestring the AMI action object.
*/
unsigned int registered:1;
/*! Number of active func() calls in progress. */
unsigned int active_count;
};
/*! \brief External routines may register/unregister manager callbacks this way

27
main/manager.c
View File

@@ -1935,7 +1935,11 @@ int ast_hook_send_action(struct manager_custom_hook *hook, const char *msg)
ao2_lock(act_found);
if (act_found->registered && act_found->func) {
++act_found->active_count;
ao2_unlock(act_found);
ret = act_found->func(&s, &m);
ao2_lock(act_found);
--act_found->active_count;
} else {
ret = -1;
}
@@ -4635,8 +4639,12 @@ static int process_message(struct mansession *s, const struct message *m)
ao2_lock(act_found);
if (act_found->registered && act_found->func) {
ast_debug(1, "Running action '%s'\n", act_found->action);
++act_found->active_count;
ao2_unlock(act_found);
ret = act_found->func(s, m);
acted = 1;
ao2_lock(act_found);
--act_found->active_count;
}
ao2_unlock(act_found);
}
@@ -5144,6 +5152,8 @@ int ast_manager_unregister(char *action)
AST_RWLIST_UNLOCK(&actions);
if (cur) {
time_t now;
/*
* We have removed the action object from the container so we
* are no longer in a hurry.
@@ -5152,6 +5162,23 @@ int ast_manager_unregister(char *action)
cur->registered = 0;
ao2_unlock(cur);
/*
* Wait up to 5 seconds for any active invocations to complete
* before returning. We have to wait instead of blocking
* because we may be waiting for ourself to complete.
*/
now = time(NULL);
while (cur->active_count) {
if (5 <= time(NULL) - now) {
ast_debug(1,
"Unregister manager action %s timed out waiting for %d active instances to complete\n",
action, cur->active_count);
break;
}
sched_yield();
}
ao2_t_ref(cur, -1, "action object removed from list");
ast_verb(2, "Manager unregistered action %s\n", action);
}