Race condition between ast_cli_command() and 'module unload' could cause a deadlock.

Add lock timeouts to avoid this potential deadlock.
(closes issue #14705)
 Reported by: jamessan
 Patches:
       20090320__bug14705.diff.txt uploaded by tilghman (license 14)
 Tested by: jamessan

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

diff --git a/include/asterisk/lock.h b/include/asterisk/lock.h
index bcf802451ede244afbe706439865d25faed43bdc..142ce5a114ae40dc3ab5301543f02052aa455f7b 100644 (file)
--- a/include/asterisk/lock.h
+++ b/include/asterisk/lock.h
@@ -993,6 +993,72 @@ static inline int _ast_rwlock_wrlock(ast_rwlock_t *lock, const char *name,
        return res;
 }
 
+#define ast_rwlock_timedrdlock(a,b) \
+       _ast_rwlock_timedrdlock(a, # a, b, __FILE__, __LINE__, __PRETTY_FUNCTION__)
+
+static inline int _ast_rwlock_rdlock(ast_rwlock_t *lock, const char *name,
+       const struct timespec *abs_timeout, const char *file, int line, const char *func)
+{
+       int res;
+#ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
+       int canlog = strcmp(file, "logger.c");
+       
+       if (*lock == ((ast_rwlock_t) AST_RWLOCK_INIT_VALUE)) {
+                /* Don't warn abount uninitialized lock.
+                 * Simple try to initialize it.
+                 * May be not needed in linux system.
+                 */
+               res = __ast_rwlock_init(file, line, func, name, lock);
+               if (*lock == ((ast_rwlock_t) AST_RWLOCK_INIT_VALUE)) {
+                       __ast_mutex_logger("%s line %d (%s): Error: rwlock '%s' is uninitialized and unable to initialize.\n",
+                                       file, line, func, name);
+                       return res;
+               }
+       }
+#endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
+       
+       ast_store_lock_info(AST_RDLOCK, file, line, func, name, lock);
+       res = pthread_rwlock_timedrdlock(lock, abs_timeout);
+       if (!res)
+               ast_mark_lock_acquired(lock);
+       else
+               ast_remove_lock_info(lock);
+       return res;
+}
+
+#define ast_rwlock_timedwrlock(a,b) \
+       _ast_rwlock_timedwrlock(a, # a, b, __FILE__, __LINE__, __PRETTY_FUNCTION__)
+
+static inline int _ast_rwlock_timedwrlock(ast_rwlock_t *lock, const char *name,
+       const struct timespec *abs_timeout, const char *file, int line, const char *func)
+{
+       int res;
+#ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
+       int canlog = strcmp(file, "logger.c");
+       
+       if (*lock == ((ast_rwlock_t) AST_RWLOCK_INIT_VALUE)) {
+                /* Don't warn abount uninitialized lock.
+                 * Simple try to initialize it.
+                 * May be not needed in linux system.
+                 */
+               res = __ast_rwlock_init(file, line, func, name, lock);
+               if (*lock == ((ast_rwlock_t) AST_RWLOCK_INIT_VALUE)) {
+                       __ast_mutex_logger("%s line %d (%s): Error: rwlock '%s' is uninitialized and unable to initialize.\n",
+                                       file, line, func, name);
+                       return res;
+               }
+       }
+#endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
+
+       ast_store_lock_info(AST_WRLOCK, file, line, func, name, lock);
+       res = pthread_rwlock_timedwrlock(lock, abs_timeout);
+       if (!res)
+               ast_mark_lock_acquired(lock);
+       else
+               ast_remove_lock_info(lock);
+       return res;
+}
+
 #define ast_rwlock_tryrdlock(a) \
        _ast_rwlock_tryrdlock(a, # a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
 
@@ -1092,6 +1158,11 @@ static inline int ast_rwlock_rdlock(ast_rwlock_t *prwlock)
        return pthread_rwlock_rdlock(prwlock);
 }
 
+static inline int ast_rwlock_timedrdlock(ast_rwlock_t *prwlock, const struct timespec *abs_timeout)
+{
+       return pthread_rwlock_timedrdlock(prwlock, abs_timeout);
+}
+
 static inline int ast_rwlock_tryrdlock(ast_rwlock_t *prwlock)
 {
        return pthread_rwlock_tryrdlock(prwlock);
@@ -1102,6 +1173,11 @@ static inline int ast_rwlock_wrlock(ast_rwlock_t *prwlock)
        return pthread_rwlock_wrlock(prwlock);
 }
 
+static inline int ast_rwlock_timedwrlock(ast_rwlock_t *prwlock, const struct timespec *abs_timeout)
+{
+       return pthread_rwlock_timedwrlock(prwlock, abs_timeout);
+}
+
 static inline int ast_rwlock_trywrlock(ast_rwlock_t *prwlock)
 {
        return pthread_rwlock_trywrlock(prwlock);

diff --git a/main/manager.c b/main/manager.c
index 54fe2080548bf428422ccc5ff391b4c2a94f9ab7..65b6d88175ffd314e7c51715865b89aad32dc402 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -2608,8 +2608,12 @@ int manager_event(int category, const char *event, const char *fmt, ...)
 int ast_manager_unregister(char *action) 
 {
        struct manager_action *cur, *prev;
+       struct timespec tv = { 5, };
 
-       ast_rwlock_wrlock(&actionlock);
+       if (ast_rwlock_timedwrlock(&actionlock, &tv)) {
+               ast_log(LOG_ERROR, "Could not obtain lock on manager list\n");
+               return -1;
+       }
        cur = prev = first_action;
        while (cur) {
                if (!strcasecmp(action, cur->action)) {
@@ -2638,8 +2642,12 @@ static int ast_manager_register_struct(struct manager_action *act)
 {
        struct manager_action *cur, *prev = NULL;
        int ret;
+       struct timespec tv = { 5, };
 
-       ast_rwlock_wrlock(&actionlock);
+       if (ast_rwlock_timedwrlock(&actionlock, &tv)) {
+               ast_log(LOG_ERROR, "Could not obtain lock on manager list\n");
+               return -1;
+       }
        cur = first_action;
        while (cur) { /* Walk the list of actions */
                ret = strcasecmp(cur->action, act->action);
@@ -2693,7 +2701,10 @@ int ast_manager_register2(const char *action, int auth, int (*func)(struct manse
        cur->description = description;
        cur->next = NULL;
 
-       ast_manager_register_struct(cur);
+       if (ast_manager_register_struct(cur)) {
+               ast_free(cur);
+               return -1;
+       }
 
        return 0;
 }