If it's possible for apps_registry to be NULL then it's possible for the condition to race. Maybe we need to use AO2_GLOBAL_OBJ_STATIC? Could this issue apply to the other global containers in this module (app_controls, app_bridges, app_bridges_moh, app_bridges_playback)? ---------- Forwarded message ---------- From: "SVN commits to the Asterisk project" < asterisk-comm...@lists.digium.com> Date: Nov 1, 2014 9:01 PM Subject: [asterisk-commits] mjordan: branch 12 r426995 - /branches/12/res/res_stasis.c To: <asterisk-comm...@lists.digium.com>, <svn-comm...@lists.digium.com> Cc:
Author: mjordan Date: Sat Nov 1 20:01:15 2014 New Revision: 426995 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=426995 Log: res/res_stasis: Fix crash on module unload while performing operation When the res_stasis module is unloaded, it will dispose of the apps_registry container. This is a problem if an ARI operation is in flight that attempts to use the registry, as the shutdown occurs in a separate thread. This patch adds some sanity checks to the various routines that access the registry which cause the operations to fail if the apps_registry does not exist. Crash caught by the Asterisk Test Suite. Modified: branches/12/res/res_stasis.c Modified: branches/12/res/res_stasis.c URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_stasis.c?view=diff&rev=426995&r1=426994&r2=426995 ============================================================================== --- branches/12/res/res_stasis.c (original) +++ branches/12/res/res_stasis.c Sat Nov 1 20:01:15 2014 @@ -1204,6 +1204,10 @@ */ remove_stasis_end_published(chan); + if (!apps_registry) { + return -1; + } + app = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY); if (!app) { ast_log(LOG_ERROR, @@ -1364,6 +1368,10 @@ { RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup); + if (!apps_registry) { + return -1; + } + app = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY); if (!app) { /* XXX We can do a better job handling late binding, queueing up @@ -1381,6 +1389,10 @@ { struct stasis_app *res = NULL; + if (!apps_registry) { + return NULL; + } + if (!ast_strlen_zero(app_name)) { res = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY); } @@ -1405,6 +1417,10 @@ { RAII_VAR(struct ao2_container *, apps, NULL, ao2_cleanup); + if (!apps_registry) { + return NULL; + } + apps = ast_str_container_alloc(1); if (!apps) { return NULL; @@ -1419,8 +1435,11 @@ { RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup); - SCOPED_LOCK(apps_lock, apps_registry, ao2_lock, ao2_unlock); - + if (!apps_registry) { + return -1; + } + + ao2_lock(apps_registry); app = ao2_find(apps_registry, app_name, OBJ_SEARCH_KEY | OBJ_NOLOCK); if (app) { app_update(app, handler, data); @@ -1429,6 +1448,7 @@ if (app) { ao2_link_flags(apps_registry, app, OBJ_NOLOCK); } else { + ao2_unlock(apps_registry); return -1; } } @@ -1437,6 +1457,7 @@ * prevent memory leaks, and we're lazy. */ cleanup(); + ao2_unlock(apps_registry); return 0; } @@ -1445,6 +1466,10 @@ RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup); if (!app_name) { + return; + } + + if (!apps_registry) { return; } @@ -1837,6 +1862,7 @@ messaging_cleanup(); + cleanup(); ao2_cleanup(apps_registry); apps_registry = NULL; -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-commits
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev