Address unload order issues for res_stasis* modules

I've noticed when doing a graceful shutdown that the res_stasis_http.so
module gets unloaded before the modules that use it, which causes some
asserts during their unload.

While r386928 was a quick hack to get it to not assert and die, this
patch increases the use counts on res_stasis.so and res_stasis_http.so
properly. It's a bigger change than I expected, hence the review instead
of just committing it.

Review: https://reviewboard.asterisk.org/r/2489/

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@388350 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/apps/app_stasis.c b/apps/app_stasis.c
index aa255ba3dea072d8f0737e7037557a1c2c333110..866e033878d2aa4075ba091c42c4dc0106ed7c19 100644 (file)
--- a/apps/app_stasis.c
+++ b/apps/app_stasis.c
@@ -93,6 +93,7 @@ static int load_module(void)
 {
        int r = 0;
 
+       stasis_app_ref();
        r |= ast_register_application_xml(stasis, app_exec);
        return r;
 }
@@ -100,12 +101,15 @@ static int load_module(void)
 static int unload_module(void)
 {
        int r = 0;
-
        r |= ast_unregister_application(stasis);
+       stasis_app_unref();
        return r;
 }
 
-AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS,
-               "Stasis dialplan application",
-               .load = load_module,
-               .unload = unload_module);
+AST_MODULE_INFO(ASTERISK_GPL_KEY,
+       AST_MODFLAG_DEFAULT,
+       "Stasis dialplan application",
+       .load = load_module,
+       .unload = unload_module,
+       .nonoptreq = "res_stasis",
+       );

diff --git a/include/asterisk/stasis_app.h b/include/asterisk/stasis_app.h
index a789e4012848e49e3c7e9831123d5d64dc9b8bcc..00d8f705fa38b67e22269679b83f31959620f5e4 100644 (file)
--- a/include/asterisk/stasis_app.h
+++ b/include/asterisk/stasis_app.h
@@ -44,6 +44,10 @@
  * Finally, Stasis apps control channels through the use of the \ref
  * stasis_app_control object, and the family of \c stasis_app_control_*
  * functions.
+ *
+ * Since module unload order is based on reference counting, any module that
+ * uses the API defined in this file must call stasis_app_ref() when loaded,
+ * and stasis_app_unref() when unloaded.
  */
 
 #include "asterisk/channel.h"
@@ -171,6 +175,20 @@ int stasis_app_control_answer(struct stasis_app_control *control);
  */
 struct ast_json *ast_channel_snapshot_to_json(const struct ast_channel_snapshot *snapshot);
 
+/*!
+ * \brief Increment the res_stasis reference count.
+ *
+ * This ensures graceful shutdown happens in the proper order.
+ */
+void stasis_app_ref(void);
+
+/*!
+ * \brief Decrement the res_stasis reference count.
+ *
+ * This ensures graceful shutdown happens in the proper order.
+ */
+void stasis_app_unref(void);
+
 /*! @} */
 
 #endif /* _ASTERISK_STASIS_APP_H */

diff --git a/main/loader.c b/main/loader.c
index cf53eda991196329fc2fbe1997e95836ef5d12bd..3bcf37ca994f7d4240131b1f2ec9f4efda4142e7 100644 (file)
--- a/main/loader.c
+++ b/main/loader.c
@@ -525,6 +525,7 @@ void ast_module_shutdown(void)
                        }
                        AST_LIST_REMOVE_CURRENT(entry);
                        if (mod->flags.running && !mod->flags.declined && mod->info->unload) {
+                               ast_verb(1, "Unloading %s\n", mod->resource);
                                mod->info->unload();
                        }
                        AST_LIST_HEAD_DESTROY(&mod->users);
@@ -571,6 +572,7 @@ int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode f
                /* Request any channels attached to the module to hangup. */
                __ast_module_user_hangup_all(mod);
 
+               ast_verb(1, "Unloading %s\n", mod->resource);
                res = mod->info->unload();
                if (res) {
                        ast_log(LOG_WARNING, "Firm unload failed for %s\n", resource_name);

diff --git a/res/res_stasis.c b/res/res_stasis.c
index a1910abeac8451953950b63a0d07307a04c7588d..b3ed1731f70957f2b13992572f9fd307269ee5c6 100644 (file)
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -942,6 +942,16 @@ static void sub_varset_handler(void *data,
        generic_blob_handler(obj, handle_blob_varset);
 }
 
+void stasis_app_ref(void)
+{
+       ast_module_ref(ast_module_info->self);
+}
+
+void stasis_app_unref(void)
+{
+       ast_module_unref(ast_module_info->self);
+}
+
 static int load_module(void)
 {
        int r = 0;

diff --git a/res/res_stasis_http.c b/res/res_stasis_http.c
index 92c9189a1e3f5695f154f166a139c491466e4c75..75a13284e4c906fbdd161f57e85fed47c31b994f 100644 (file)
--- a/res/res_stasis_http.c
+++ b/res/res_stasis_http.c
@@ -72,7 +72,6 @@
  */
 
 /*** MODULEINFO
-       <depend type="module">app_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -235,6 +234,7 @@ int stasis_http_add_handler(struct stasis_rest_handlers *handler)
        ao2_cleanup(root_handler);
        ao2_ref(new_handler, +1);
        root_handler = new_handler;
+       ast_module_ref(ast_module_info->self);
        return 0;
 }
 
@@ -243,9 +243,7 @@ int stasis_http_remove_handler(struct stasis_rest_handlers *handler)
        RAII_VAR(struct stasis_rest_handlers *, new_handler, NULL, ao2_cleanup);
        size_t size, i, j;
 
-       if (!root_handler) {
-               return -1;
-       }
+       ast_assert(root_handler != NULL);
 
        ast_mutex_lock(&root_handler_lock);
        size = sizeof(*new_handler) +
@@ -259,6 +257,7 @@ int stasis_http_remove_handler(struct stasis_rest_handlers *handler)
 
        for (i = 0, j = 0; i < root_handler->num_children; ++i) {
                if (root_handler->children[i] == handler) {
+                       ast_module_unref(ast_module_info->self);
                        continue;
                }
                new_handler->children[j++] = root_handler->children[i];
@@ -860,6 +859,7 @@ static struct ast_http_uri http_uri = {
 static int load_module(void)
 {
        ast_mutex_init(&root_handler_lock);
+
        root_handler = root_handler_create();
        if (!root_handler) {
                return AST_MODULE_LOAD_FAILURE;
@@ -932,6 +932,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY,
        .load = load_module,
        .unload = unload_module,
        .reload = reload_module,
-       .nonoptreq = "app_stasis",
        .load_pri = AST_MODPRI_APP_DEPEND,
        );

diff --git a/res/res_stasis_http_asterisk.c b/res/res_stasis_http_asterisk.c
index 333c66b5c6746080654e440f550de6b43af2d57b..f8b6ea767e97d80233cde6eac17859d7b8ef9908 100644 (file)
--- a/res/res_stasis_http_asterisk.c
+++ b/res/res_stasis_http_asterisk.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_asterisk.h"
 
 /*!
@@ -86,12 +88,14 @@ static struct stasis_rest_handlers asterisk = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&asterisk);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&asterisk);
+       stasis_app_unref();
        return 0;
 }
 
@@ -99,5 +103,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Asterisk resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_bridges.c b/res/res_stasis_http_bridges.c
index 48d5b72774ed2832ace9c4f9a87083cd87afa78a..a2301195c9ec3443e2f75b34be0b42acfcde43c1 100644 (file)
--- a/res/res_stasis_http_bridges.c
+++ b/res/res_stasis_http_bridges.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_bridges.h"
 
 /*!
@@ -274,12 +276,14 @@ static struct stasis_rest_handlers bridges = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&bridges);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&bridges);
+       stasis_app_unref();
        return 0;
 }
 
@@ -287,5 +291,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Bridge resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_channels.c b/res/res_stasis_http_channels.c
index c21bc10739d6955cbc7c778b4c6febaf4028704a..aa4481903451ac45edd61a6fa8c094677ca9a8ad 100644 (file)
--- a/res/res_stasis_http_channels.c
+++ b/res/res_stasis_http_channels.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_channels.h"
 
 /*!
@@ -487,12 +489,14 @@ static struct stasis_rest_handlers channels = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&channels);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&channels);
+       stasis_app_unref();
        return 0;
 }
 
@@ -500,5 +504,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Channel resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_endpoints.c b/res/res_stasis_http_endpoints.c
index a09be1b6e4e583307e16f2b726f8201f2f218c69..e05031e638eef014b6b669d30b1eb23c1b2d6306 100644 (file)
--- a/res/res_stasis_http_endpoints.c
+++ b/res/res_stasis_http_endpoints.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_endpoints.h"
 
 /*!
@@ -137,12 +139,14 @@ static struct stasis_rest_handlers endpoints = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&endpoints);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&endpoints);
+       stasis_app_unref();
        return 0;
 }
 
@@ -150,5 +154,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Endpoint resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_events.c b/res/res_stasis_http_events.c
index 98d845d586e9b35914bfb6e7c3a93a1ba11d7fac..3b2a9735f3ec4a2536dc8662fe6f33828fc8b7c1 100644 (file)
--- a/res/res_stasis_http_events.c
+++ b/res/res_stasis_http_events.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_events.h"
 #include "asterisk/stasis_channels.h"
 
@@ -597,12 +599,14 @@ struct ast_json *stasis_json_event_stasis_end_create(
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&events);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&events);
+       stasis_app_unref();
        return 0;
 }
 
@@ -610,5 +614,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - WebSocket resource",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_playback.c b/res/res_stasis_http_playback.c
index 77dcee41ccd9ceb83df33cd24a83927c0f06324c..ad16de11dd8d83b6ce0428de85a143454cae5ff1 100644 (file)
--- a/res/res_stasis_http_playback.c
+++ b/res/res_stasis_http_playback.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_playback.h"
 
 /*!
@@ -147,12 +149,14 @@ static struct stasis_rest_handlers playback = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&playback);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&playback);
+       stasis_app_unref();
        return 0;
 }
 
@@ -160,5 +164,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Playback control resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_recordings.c b/res/res_stasis_http_recordings.c
index b6a3b418e43ce57c73712ad33cce14a792621006..687dec4ede3e28766166645529c7e9fa2863d832 100644 (file)
--- a/res/res_stasis_http_recordings.c
+++ b/res/res_stasis_http_recordings.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_recordings.h"
 
 /*!
@@ -381,12 +383,14 @@ static struct stasis_rest_handlers recordings = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&recordings);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&recordings);
+       stasis_app_unref();
        return 0;
 }
 
@@ -394,5 +398,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Recording resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_http_sounds.c b/res/res_stasis_http_sounds.c
index 39ad71e5263fa4eccd4364fb18830e45378b6c56..ef761011e6651d9c822cbcb9d4d7f52ea951c43f 100644 (file)
--- a/res/res_stasis_http_sounds.c
+++ b/res/res_stasis_http_sounds.c
@@ -33,6 +33,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -41,6 +42,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_sounds.h"
 
 /*!
@@ -113,12 +115,14 @@ static struct stasis_rest_handlers sounds = {
 
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&sounds);
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&sounds);
+       stasis_app_unref();
        return 0;
 }
 
@@ -126,5 +130,5 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - Sound resources",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );

diff --git a/res/res_stasis_websocket.c b/res/res_stasis_websocket.c
index 4cf5b94986046209f7fb2a35e425d23c37524213..50fec06251d9b26ec190423671d26a6a5b9d10af 100644 (file)
--- a/res/res_stasis_websocket.c
+++ b/res/res_stasis_websocket.c
@@ -263,6 +263,7 @@ static int load_module(void)
 {
        int r = 0;
 
+       stasis_app_ref();
        oom_json = ast_json_pack("{s: s}",
                                 "error", "OutOfMemory");
        if (!oom_json) {
@@ -277,6 +278,7 @@ static int unload_module(void)
 {
        int r = 0;
 
+       stasis_app_unref();
        ast_json_unref(oom_json);
        oom_json = NULL;
        r |= ast_websocket_remove_protocol(ws_protocol, websocket_callback);

diff --git a/rest-api-templates/res_stasis_http_resource.c.mustache b/rest-api-templates/res_stasis_http_resource.c.mustache
index 9b1324c0917360bce5e286d0addda709577a3bba..a9428c0607d7ffd041e6dffb4862e7d49a23ee6f 100644 (file)
--- a/rest-api-templates/res_stasis_http_resource.c.mustache
+++ b/rest-api-templates/res_stasis_http_resource.c.mustache
@@ -38,6 +38,7 @@
 
 /*** MODULEINFO
        <depend type="module">res_stasis_http</depend>
+       <depend type="module">res_stasis</depend>
        <support_level>core</support_level>
  ***/
 
@@ -46,6 +47,7 @@
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #include "asterisk/module.h"
+#include "asterisk/stasis_app.h"
 #include "stasis_http/resource_{{name}}.h"
 {{#has_events}}
 #include "asterisk/stasis_channels.h"
@@ -184,12 +186,14 @@ static void stasis_http_{{c_nickname}}_cb(
 {{/has_events}}
 static int load_module(void)
 {
+       stasis_app_ref();
        return stasis_http_add_handler(&{{root_full_name}});
 }
 
 static int unload_module(void)
 {
        stasis_http_remove_handler(&{{root_full_name}});
+       stasis_app_unref();
        return 0;
 }
 
@@ -197,6 +201,6 @@ AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_DEFAULT,
        "RESTful API module - {{{description}}}",
        .load = load_module,
        .unload = unload_module,
-       .nonoptreq = "res_stasis_http",
+       .nonoptreq = "res_stasis_http,res_stasis",
        );
 {{/api_declaration}}

diff --git a/tests/test_res_stasis.c b/tests/test_res_stasis.c
index 0e7daf5dde5bbdf9cc3bba1486041d2edd2ff2fd..a824b8a8daa7b7e96f1e35cf9dc0cbbfd9e47aea 100644 (file)
--- a/tests/test_res_stasis.c
+++ b/tests/test_res_stasis.c
@@ -176,11 +176,13 @@ static int unload_module(void)
        AST_TEST_UNREGISTER(app_invoke_dne);
        AST_TEST_UNREGISTER(app_invoke_one);
        AST_TEST_UNREGISTER(app_replaced);
+       stasis_app_unref();
        return 0;
 }
 
 static int load_module(void)
 {
+       stasis_app_ref();
        AST_TEST_REGISTER(app_replaced);
        AST_TEST_REGISTER(app_invoke_one);
        AST_TEST_REGISTER(app_invoke_dne);