From c9648f4690df2e8e23e60ffa70d4e9813246b62b Mon Sep 17 00:00:00 2001
From: Sean Bright <sean.bright@gmail.com>
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 | 8 ++++----
main/astobj2.c | 24 +-----------------------
2 files changed, 5 insertions(+), 27 deletions(-)
diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h
index 4bd44db763..390e0ea1b6 100644
--- a/include/asterisk/astobj2.h
+++ b/include/asterisk/astobj2.h
@@ -741,16 +741,16 @@ struct ao2_global_obj {
*/
#ifdef REF_DEBUG
#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)
#else
#define ao2_t_global_obj_release(holder, tag) \
- __ao2_global_obj_release(&holder, NULL, __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder)
+ __ao2_global_obj_replace_unref(&holder, NULL, NULL, __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder)
#define ao2_global_obj_release(holder) \
- __ao2_global_obj_release(&holder, NULL, __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder)
+ __ao2_global_obj_replace_unref(&holder, NULL, NULL, __FILE__, __LINE__, __PRETTY_FUNCTION__, #holder)
#endif
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 569db0b7b5..147df7aedc 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -635,29 +635,7 @@ void *__ao2_alloc(size_t data_size, ao2_destructor_fn destructor_fn, unsigned in
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) {
- if (tag) {
- __ao2_ref_debug(holder->obj, -1, tag, file, line, func);
- } else {
- __ao2_ref(holder->obj, -1);
- }
- 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)
--
GitLab