Merged revisions 187483 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

................
  r187483 | tilghman | 2009-04-09 13:40:01 -0500 (Thu, 09 Apr 2009) | 15 lines

  Merged revisions 187428 via svnmerge from
  https://origsvn.digium.com/svn/asterisk/branches/1.4

  ........
    r187428 | tilghman | 2009-04-09 13:08:20 -0500 (Thu, 09 Apr 2009) | 8 lines

    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.6.1@187486 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/linkedlists.h b/include/asterisk/linkedlists.h
index 1db0ae2db1ed6a37512a2f481b7d86d5f8862b6e..dd5e2f6ca03cc055fb499cce8d4709f72759211a 100644 (file)
--- a/include/asterisk/linkedlists.h
+++ b/include/asterisk/linkedlists.h
@@ -51,6 +51,18 @@
 #define AST_RWLIST_WRLOCK(head)                                         \
         ast_rwlock_wrlock(&(head)->lock)
 
+/*!
+  \brief Write locks a list, with timeout.
+  \param head This is a pointer to the list head structure
+  \param tv Pointer to a timeval structure
+
+  This macro attempts to place an exclusive write lock in the
+  list head structure pointed to by head.
+  \retval 0 on success
+  \retval non-zero on failure
+*/
+#define        AST_RWLIST_TIMEDWRLOCK(head,tv) ast_rwlock_timedwrlock(&(head)->lock, tv)
+
 /*!
   \brief Read locks a list.
   \param head This is a pointer to the list head structure
@@ -63,6 +75,19 @@
 #define AST_RWLIST_RDLOCK(head)                                         \
         ast_rwlock_rdlock(&(head)->lock)
 
+/*!
+  \brief Read locks a list, with timeout.
+  \param head This is a pointer to the list head structure
+  \param tv Pointer to a timeval structure
+
+  This macro attempts to place a read lock in the
+  list head structure pointed to by head.
+  \retval 0 on success
+  \retval non-zero on failure
+*/
+#define AST_RWLIST_TIMEDRDLOCK(head,tv)                                 \
+        ast_rwlock_timedrdlock(&(head)->lock, tv)
+
 /*!
   \brief Locks a list, without blocking if the list is locked.
   \param head This is a pointer to the list head structure

diff --git a/include/asterisk/lock.h b/include/asterisk/lock.h
index 643bf1c6030edee624900d5738b4cd24234bc4ef..139f54cad0a599aaad5832b52a9203a58250938e 100644 (file)
--- a/include/asterisk/lock.h
+++ b/include/asterisk/lock.h
@@ -1353,6 +1353,162 @@ static inline int _ast_rwlock_wrlock(ast_rwlock_t *t, 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_timedrdlock(ast_rwlock_t *t, const char *name,
+       const struct timespec *abs_timeout, const char *filename, int line, const char *func)
+{
+       int res;
+       struct ast_lock_track *lt = &t->track;
+       int canlog = strcmp(filename, "logger.c") & t->tracking;
+#ifdef HAVE_BKTR
+       struct ast_bt *bt = NULL;
+#endif
+
+#ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
+       if ((t->lock) == ((pthread_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(t->tracking, filename, line, func, name, t);
+               if ((t->lock) == ((pthread_rwlock_t) __AST_RWLOCK_INIT_VALUE)) {
+                       __ast_mutex_logger("%s line %d (%s): Error: rwlock '%s' is uninitialized and unable to initialize.\n",
+                                       filename, line, func, name);
+                       return res;
+               }
+       }
+#endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
+
+       if (t->tracking) {
+#ifdef HAVE_BKTR
+               ast_reentrancy_lock(lt);
+               if (lt->reentrancy != AST_MAX_REENTRANCY) {
+                       ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+                       bt = &lt->backtrace[lt->reentrancy];
+               }
+               ast_reentrancy_unlock(lt);
+               ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
+#else
+               ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);
+#endif
+       }
+       res = pthread_rwlock_timedrdlock(&t->lock, abs_timeout);
+       if (!res) {
+               ast_reentrancy_lock(lt);
+               if (lt->reentrancy < AST_MAX_REENTRANCY) {
+                       lt->file[lt->reentrancy] = filename;
+                       lt->lineno[lt->reentrancy] = line;
+                       lt->func[lt->reentrancy] = func;
+                       lt->thread[lt->reentrancy] = pthread_self();
+                       lt->reentrancy++;
+               }
+               ast_reentrancy_unlock(lt);
+               if (t->tracking) {
+                       ast_mark_lock_acquired(t);
+               }
+       } else {
+#ifdef HAVE_BKTR
+               if (lt->reentrancy) {
+                       ast_reentrancy_lock(lt);
+                       bt = &lt->backtrace[lt->reentrancy-1];
+               } else {
+                       bt = NULL;
+               }
+               if (t->tracking) {
+                       ast_remove_lock_info(t, bt);
+               }
+#else
+               if (t->tracking) {
+                       ast_remove_lock_info(t);
+               }
+#endif
+               __ast_mutex_logger("%s line %d (%s): Error obtaining read lock: %s\n",
+                               filename, line, func, strerror(res));
+               DO_THREAD_CRASH;
+       }
+       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 *t, const char *name,
+       const struct timespec *abs_timeout, const char *filename, int line, const char *func)
+{
+       int res;
+       struct ast_lock_track *lt = &t->track;
+       int canlog = strcmp(filename, "logger.c") & t->tracking;
+#ifdef HAVE_BKTR
+       struct ast_bt *bt = NULL;
+#endif
+
+#ifdef AST_MUTEX_INIT_W_CONSTRUCTORS
+       if ((t->lock) == ((pthread_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(t->tracking, filename, line, func, name, t);
+               if ((t->lock) == ((pthread_rwlock_t) __AST_RWLOCK_INIT_VALUE)) {
+                       __ast_mutex_logger("%s line %d (%s): Error: rwlock '%s' is uninitialized and unable to initialize.\n",
+                                       filename, line, func, name);
+                       return res;
+               }
+       }
+#endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
+
+       if (t->tracking) {
+#ifdef HAVE_BKTR
+               ast_reentrancy_lock(lt);
+               if (lt->reentrancy != AST_MAX_REENTRANCY) {
+                       ast_bt_get_addresses(&lt->backtrace[lt->reentrancy]);
+                       bt = &lt->backtrace[lt->reentrancy];
+               }
+               ast_reentrancy_unlock(lt);
+               ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t, bt);
+#else
+               ast_store_lock_info(AST_WRLOCK, filename, line, func, name, t);
+#endif
+       }
+       res = pthread_rwlock_timedwrlock(&t->lock, abs_timeout);
+       if (!res) {
+               ast_reentrancy_lock(lt);
+               if (lt->reentrancy < AST_MAX_REENTRANCY) {
+                       lt->file[lt->reentrancy] = filename;
+                       lt->lineno[lt->reentrancy] = line;
+                       lt->func[lt->reentrancy] = func;
+                       lt->thread[lt->reentrancy] = pthread_self();
+                       lt->reentrancy++;
+               }
+               ast_reentrancy_unlock(lt);
+               if (t->tracking) {
+                       ast_mark_lock_acquired(t);
+               }
+       } else {
+#ifdef HAVE_BKTR
+               if (lt->reentrancy) {
+                       ast_reentrancy_lock(lt);
+                       bt = &lt->backtrace[lt->reentrancy-1];
+               } else {
+                       bt = NULL;
+               }
+               if (t->tracking) {
+                       ast_remove_lock_info(t, bt);
+               }
+#else
+               if (t->tracking) {
+                       ast_remove_lock_info(t);
+               }
+#endif
+               __ast_mutex_logger("%s line %d (%s): Error obtaining read lock: %s\n",
+                               filename, line, func, strerror(res));
+               DO_THREAD_CRASH;
+       }
+       return res;
+}
+
 static inline int _ast_rwlock_tryrdlock(ast_rwlock_t *t, const char *name,
        const char *filename, int line, const char *func)
 {
@@ -1601,6 +1757,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);
@@ -1611,6 +1772,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 92cf462d7a61905c66878dcf2cb03370dd78e55d..eb7e43f5fc74a7cf0f2c8994c4e3c2d42fc830ce 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -3338,8 +3338,12 @@ int __manager_event(int category, const char *event,
 int ast_manager_unregister(char *action)
 {
        struct manager_action *cur;
+       struct timespec tv = { 5, };
 
-       AST_RWLIST_WRLOCK(&actions);
+       if (AST_RWLIST_TIMEDWRLOCK(&actions, &tv)) {
+               ast_log(LOG_ERROR, "Could not obtain lock on manager list\n");
+               return -1;
+       }
        AST_RWLIST_TRAVERSE_SAFE_BEGIN(&actions, cur, list) {
                if (!strcasecmp(action, cur->action)) {
                        AST_RWLIST_REMOVE_CURRENT(list);
@@ -3348,7 +3352,7 @@ int ast_manager_unregister(char *action)
                        break;
                }
        }
-       AST_RWLIST_TRAVERSE_SAFE_END;
+       AST_RWLIST_TRAVERSE_SAFE_END
        AST_RWLIST_UNLOCK(&actions);
 
        return 0;
@@ -3367,8 +3371,12 @@ static int manager_state_cb(char *context, char *exten, int state, void *data)
 static int ast_manager_register_struct(struct manager_action *act)
 {
        struct manager_action *cur, *prev = NULL;
+       struct timespec tv = { 5, };
 
-       AST_RWLIST_WRLOCK(&actions);
+       if (AST_RWLIST_TIMEDWRLOCK(&actions, &tv)) {
+               ast_log(LOG_ERROR, "Could not obtain lock on manager list\n");
+               return -1;
+       }
        AST_RWLIST_TRAVERSE(&actions, cur, list) {
                int ret = strcasecmp(cur->action, act->action);
                if (ret == 0) {
@@ -3381,8 +3389,8 @@ static int ast_manager_register_struct(struct manager_action *act)
                        break;
                }
        }
-       
-       if (prev)       
+
+       if (prev)
                AST_RWLIST_INSERT_AFTER(&actions, prev, act, list);
        else
                AST_RWLIST_INSERT_HEAD(&actions, act, list);
@@ -3409,7 +3417,10 @@ int ast_manager_register2(const char *action, int auth, int (*func)(struct manse
        cur->synopsis = synopsis;
        cur->description = description;
 
-       ast_manager_register_struct(cur);
+       if (ast_manager_register_struct(cur)) {
+               ast_free(cur);
+               return -1;
+       }
 
        return 0;
 }