From b3fe94e4cac8b6f871e3873b529c62ca54c2600c Mon Sep 17 00:00:00 2001 From: Sean Bright Date: Wed, 29 Mar 2017 11:11:51 -0400 Subject: [PATCH] astobj2: Prevent potential deadlocks with ao2_global_obj_release The ao2_global_obj_release() function holds an exclusive lock on the global object while it is being dereferenced. Any destructors that run during this time that call ao2_global_obj_ref() will deadlock because a read lock is required. Instead, we make the global object inaccessible inside of the write lock and only dereference it once we have released the lock. This allows the affected destructors to fail gracefully. While this doesn't completely solve the referenced issue (the error message about not being able to create an IQ continues to be shown) it does solve the backtrace spew that accompanied it. ASTERISK-21009 #close Reported by: Marcello Ceschia Change-Id: Idf40ae136b5070dba22cb576ea8414fbc9939385 --- include/asterisk/astobj2.h | 4 ++-- main/astobj2.c | 20 +------------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h index 28ae73e87e..33e022421b 100644 --- a/include/asterisk/astobj2.h +++ b/include/asterisk/astobj2.h @@ -854,9 +854,9 @@ struct ao2_global_obj { * \return Nothing */ #define ao2_t_global_obj_release(holder, tag) \ - __ao2_global_obj_release(&holder, (tag), __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder) + __ao2_global_obj_replace_unref(&holder, NULL, (tag), __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder) #define ao2_global_obj_release(holder) \ - __ao2_global_obj_release(&holder, "", __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder) + __ao2_global_obj_replace_unref(&holder, NULL, "", __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder) void __ao2_global_obj_release(struct ao2_global_obj *holder, const char *tag, const char *file, int line, const char *func, const char *name); diff --git a/main/astobj2.c b/main/astobj2.c index 71a81eacfa..1702f514c8 100644 --- a/main/astobj2.c +++ b/main/astobj2.c @@ -750,25 +750,7 @@ unsigned int ao2_options_get(void *obj) void __ao2_global_obj_release(struct ao2_global_obj *holder, const char *tag, const char *file, int line, const char *func, const char *name) { - if (!holder) { - /* For sanity */ - ast_log(LOG_ERROR, "Must be called with a global object!\n"); - ast_assert(0); - return; - } - if (__ast_rwlock_wrlock(file, line, func, &holder->lock, name)) { - /* Could not get the write lock. */ - ast_assert(0); - return; - } - - /* Release the held ao2 object. */ - if (holder->obj) { - __ao2_ref(holder->obj, -1, tag, file, line, func); - holder->obj = NULL; - } - - __ast_rwlock_unlock(file, line, func, &holder->lock, name); + __ao2_global_obj_replace_unref(holder, NULL, tag, file, line, func, name); } void *__ao2_global_obj_replace(struct ao2_global_obj *holder, void *obj, const char *tag, const char *file, int line, const char *func, const char *name) -- 2.47.2