From fefac6b6c0a307cdb6dffd2d1360f0a212bfac8a Mon Sep 17 00:00:00 2001 From: tilghman Date: Thu, 9 Apr 2009 18:40:01 +0000 Subject: [PATCH] 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: http://svn.digium.com/svn/asterisk/trunk@187483 f38db490-d61c-443f-a65b-d21fe96a405b --- include/asterisk/linkedlists.h | 25 +++++ include/asterisk/lock.h | 166 +++++++++++++++++++++++++++++++++ main/manager.c | 23 +++-- 3 files changed, 208 insertions(+), 6 deletions(-) diff --git a/include/asterisk/linkedlists.h b/include/asterisk/linkedlists.h index 1db0ae2db..dd5e2f6ca 100644 --- 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 643bf1c60..139f54cad 100644 --- 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(<->backtrace[lt->reentrancy]); + bt = <->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 = <->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(<->backtrace[lt->reentrancy]); + bt = <->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 = <->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 8987034cd..105274cba 100644 --- a/main/manager.c +++ b/main/manager.c @@ -3367,8 +3367,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); @@ -3377,7 +3381,7 @@ int ast_manager_unregister(char *action) break; } } - AST_RWLIST_TRAVERSE_SAFE_END; + AST_RWLIST_TRAVERSE_SAFE_END AST_RWLIST_UNLOCK(&actions); return 0; @@ -3396,8 +3400,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) { @@ -3410,8 +3418,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); @@ -3438,7 +3446,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; }