stasis: No need to keep a stasis type ref in a stasis msg or cache object.

Stasis message types are global ao2 objects and we make stasis messages
and cache entries hold references to them.  Since there are currently
situations where cache objects are never deleted, the reference count on
the types can exceed 100000 and generate a FRACK assertion message.  The
stasis message cache could conceivably also have that many messages
legitimately on large systems.

The only down side to not holding the message type ref in the stasis
message is it only makes a crash either at shutdown or when manually
unloading a busy module slightly more likely.  However, this is more
exposing a pre-existing stasis shutdown ordering issue than a problem with
not holding a message type ref in stasis messages.

* Made stasis messages and cache entries no longer hold a ref to the
message type.

Change-Id: Ibaa28efa8d8ad3836f0c65957192424c7f561707

diff --git a/main/stasis_cache.c b/main/stasis_cache.c
index 9907c6c..dfb154d 100644 (file)
--- a/main/stasis_cache.c
+++ b/main/stasis_cache.c
@@ -156,7 +156,6 @@ static void cache_entry_dtor(void *obj)
        struct stasis_cache_entry *entry = obj;
        size_t idx;
 
-       ao2_cleanup(entry->key.type);
        entry->key.type = NULL;
        ast_free((char *) entry->key.id);
        entry->key.id = NULL;
@@ -204,7 +203,16 @@ static struct stasis_cache_entry *cache_entry_create(struct stasis_message_type
                ao2_cleanup(entry);
                return NULL;
        }
-       entry->key.type = ao2_bump(type);
+       /*
+        * Normal ao2 ref counting rules says we should increment the message
+        * type ref here and decrement it in cache_entry_dtor().  However, the
+        * stasis message snapshot is cached here, will always have the same type
+        * as the cache entry, and can legitimately cause the type ref count to
+        * hit the excessive ref count assertion.  Since the cache entry will
+        * always have a snapshot we can get away with not holding a ref here.
+        */
+       ast_assert(type == stasis_message_type(snapshot));
+       entry->key.type = type;
        cache_entry_compute_hash(&entry->key);
 
        is_remote = ast_eid_cmp(&ast_eid_default, stasis_message_eid(snapshot)) ? 1 : 0;

diff --git a/main/stasis_message.c b/main/stasis_message.c
index 49d6c05..19f4a92 100644 (file)
--- a/main/stasis_message.c
+++ b/main/stasis_message.c
@@ -110,7 +110,6 @@ struct stasis_message {
 static void stasis_message_dtor(void *obj)
 {
        struct stasis_message *message = obj;
-       ao2_cleanup(message->type);
        ao2_cleanup(message->data);
 }
 
@@ -129,7 +128,14 @@ struct stasis_message *stasis_message_create_full(struct stasis_message_type *ty
        }
 
        message->timestamp = ast_tvnow();
-       ao2_ref(type, +1);
+       /*
+        * XXX Normal ao2 ref counting rules says we should increment the message
+        * type ref here and decrement it in stasis_message_dtor().  However, the
+        * stasis message could be cached and legitimately cause the type ref count
+        * to hit the excessive ref count assertion.  Since the message type
+        * practically has to be a global object anyway, we can get away with not
+        * holding a ref in the stasis message.
+        */
        message->type = type;
        ao2_ref(data, +1);
        message->data = data;