[svn-commits] kharwell: branch 13 r430628 - in /branches/13: main/ res/ res/res_pjsip/ res/...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Wed Jan 14 17:14:51 CST 2015


Author: kharwell
Date: Wed Jan 14 17:14:47 2015
New Revision: 430628

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=430628
Log:
res_pjsip: make it unloadable

The res_pjsip module was previously unloadable. With this patch it can now
be unloaded.

This patch is based off the original patch on the issue (listed below) by Corey
Farrell with a few modifications. Namely, removed a few changes not required to
make the module unloadable and also fixed a bug that would cause asterisk to
crash on unloading.

This patch is the first step (should hopefully be followed by another/others at
some point) in allowing res_pjsip and the modules that depend on it to be
unloadable. At this time, res_pjsip and some of the modules that depend on
res_pjsip cannot be unloaded without causing problems of some sort.

The goal of this patch is to get res_pjsip and only res_pjsip to be able to
unload successfully and/or shutdown without incident (crashes, leaks, etc...).
Other dependent modules may still cause problems on unload.

Basically made sure, with the patch applied, that res_pjsip (with no other
dependent modules loaded) could be succesfully unloaded and Asterisk could
shutdown without any leaks or crashes that pertained directly to res_pjsip.

ASTERISK-24485 #close
Reported by: Corey Farrell
Review: https://reviewboard.asterisk.org/r/4311/
patches:
  pjsip_unload-broken-r1.patch submitted by Corey Farrell (license 5909)

Modified:
    branches/13/main/stasis_message_router.c
    branches/13/res/res_pjsip.c
    branches/13/res/res_pjsip/config_auth.c
    branches/13/res/res_pjsip/config_transport.c
    branches/13/res/res_pjsip/include/res_pjsip_private.h
    branches/13/res/res_pjsip/location.c
    branches/13/res/res_pjsip/pjsip_configuration.c
    branches/13/res/res_pjsip/pjsip_distributor.c
    branches/13/res/res_pjsip/pjsip_global_headers.c
    branches/13/res/res_pjsip/pjsip_options.c
    branches/13/res/res_pjsip/pjsip_outbound_auth.c

Modified: branches/13/main/stasis_message_router.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/main/stasis_message_router.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/main/stasis_message_router.c (original)
+++ branches/13/main/stasis_message_router.c Wed Jan 14 17:14:47 2015
@@ -255,7 +255,7 @@
 		return;
 	}
 
-	stasis_unsubscribe(router->subscription);
+	router->subscription = stasis_unsubscribe(router->subscription);
 }
 
 void stasis_message_router_unsubscribe_and_join(

Modified: branches/13/res/res_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip.c (original)
+++ branches/13/res/res_pjsip.c Wed Jan 14 17:14:47 2015
@@ -40,6 +40,8 @@
 /*** MODULEINFO
 	<depend>pjproject</depend>
 	<depend>res_sorcery_config</depend>
+	<depend>res_sorcery_memory</depend>
+	<depend>res_sorcery_astdb</depend>
 	<support_level>core</support_level>
  ***/
 
@@ -1799,7 +1801,7 @@
 
 static struct ast_threadpool *sip_threadpool;
 
-static int register_service(void *data)
+static int register_service_noref(void *data)
 {
 	pjsip_module **module = data;
 	if (!ast_pjsip_endpoint) {
@@ -1811,25 +1813,55 @@
 		return -1;
 	}
 	ast_debug(1, "Registered SIP service %.*s (%p)\n", (int) pj_strlen(&(*module)->name), pj_strbuf(&(*module)->name), *module);
-	ast_module_ref(ast_module_info->self);
 	return 0;
 }
 
+static int register_service(void *data)
+{
+	int res;
+
+	if (!(res = register_service_noref(data))) {
+		ast_module_ref(ast_module_info->self);
+	}
+
+	return res;
+}
+
+int internal_sip_register_service(pjsip_module *module)
+{
+	return ast_sip_push_task_synchronous(NULL, register_service_noref, &module);
+}
+
 int ast_sip_register_service(pjsip_module *module)
 {
 	return ast_sip_push_task_synchronous(NULL, register_service, &module);
 }
 
-static int unregister_service(void *data)
+static int unregister_service_noref(void *data)
 {
 	pjsip_module **module = data;
-	ast_module_unref(ast_module_info->self);
 	if (!ast_pjsip_endpoint) {
 		return -1;
 	}
 	pjsip_endpt_unregister_module(ast_pjsip_endpoint, *module);
 	ast_debug(1, "Unregistered SIP service %.*s\n", (int) pj_strlen(&(*module)->name), pj_strbuf(&(*module)->name));
 	return 0;
+}
+
+static int unregister_service(void *data)
+{
+	int res;
+
+	if (!(res = unregister_service_noref(data))) {
+		ast_module_unref(ast_module_info->self);
+	}
+
+	return res;
+}
+
+int internal_sip_unregister_service(pjsip_module *module)
+{
+	return ast_sip_push_task_synchronous(NULL, unregister_service_noref, &module);
 }
 
 void ast_sip_unregister_service(pjsip_module *module)
@@ -1978,26 +2010,38 @@
 
 AST_RWLIST_HEAD_STATIC(endpoint_formatters, ast_sip_endpoint_formatter);
 
-int ast_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
 {
 	SCOPED_LOCK(lock, &endpoint_formatters, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
 	AST_RWLIST_INSERT_TAIL(&endpoint_formatters, obj, next);
+}
+
+int ast_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+{
+	internal_sip_register_endpoint_formatter(obj);
 	ast_module_ref(ast_module_info->self);
 	return 0;
 }
 
-void ast_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
 {
 	struct ast_sip_endpoint_formatter *i;
 	SCOPED_LOCK(lock, &endpoint_formatters, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
 	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&endpoint_formatters, i, next) {
 		if (i == obj) {
 			AST_RWLIST_REMOVE_CURRENT(next);
-			ast_module_unref(ast_module_info->self);
 			break;
 		}
 	}
 	AST_RWLIST_TRAVERSE_SAFE_END;
+	return i == obj ? 0 : -1;
+}
+
+void ast_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+{
+	if (!internal_sip_unregister_endpoint_formatter(obj)) {
+		ast_module_unref(ast_module_info->self);
+	}
 }
 
 int ast_sip_format_endpoint_ami(struct ast_sip_endpoint *endpoint,
@@ -3206,7 +3250,7 @@
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	if (ast_sip_register_service(&supplement_module)) {
+	if (internal_sip_register_service(&supplement_module)) {
 		ast_log(LOG_ERROR, "Failed to initialize supplement hooks. Aborting load\n");
 		ast_sip_destroy_distributor();
 		ast_res_pjsip_destroy_configuration();
@@ -3221,9 +3265,9 @@
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	if (ast_sip_initialize_outbound_authentication()) {
+	if (internal_sip_initialize_outbound_authentication()) {
 		ast_log(LOG_ERROR, "Failed to initialize outbound authentication. Aborting load\n");
-		ast_sip_unregister_service(&supplement_module);
+		internal_sip_unregister_service(&supplement_module);
 		ast_sip_destroy_distributor();
 		ast_res_pjsip_destroy_configuration();
 		ast_sip_destroy_global_headers();
@@ -3239,8 +3283,6 @@
 
 	ast_res_pjsip_init_options_handling(0);
 
-	ast_module_ref(ast_module_info->self);
-
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
@@ -3254,9 +3296,41 @@
 	return 0;
 }
 
+static int unload_pjsip(void *data)
+{
+	if (memory_pool) {
+		pj_pool_release(memory_pool);
+		memory_pool = NULL;
+	}
+	if (ast_pjsip_endpoint) {
+		pjsip_endpt_destroy(ast_pjsip_endpoint);
+		ast_pjsip_endpoint = NULL;
+	}
+	pj_caching_pool_destroy(&caching_pool);
+	pj_shutdown();
+	return 0;
+}
+
 static int unload_module(void)
 {
-	/* This will never get called as this module can't be unloaded */
+	ast_res_pjsip_cleanup_options_handling();
+	internal_sip_destroy_outbound_authentication();
+	ast_sip_destroy_distributor();
+	ast_res_pjsip_destroy_configuration();
+	ast_sip_destroy_system();
+	ast_sip_destroy_global_headers();
+	internal_sip_unregister_service(&supplement_module);
+	if (monitor_thread) {
+		stop_monitor_thread();
+	}
+	/* The thread this is called from cannot call PJSIP/PJLIB functions,
+	 * so we have to push the work to the threadpool to handle
+	 */
+	ast_sip_push_task_synchronous(NULL, unload_pjsip, NULL);
+
+	ast_threadpool_shutdown(sip_threadpool);
+
+	ast_sip_destroy_cli();
 	return 0;
 }
 

Modified: branches/13/res/res_pjsip/config_auth.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/config_auth.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/config_auth.c (original)
+++ branches/13/res/res_pjsip/config_auth.c Wed Jan 14 17:14:47 2015
@@ -312,7 +312,7 @@
 	ast_sorcery_object_field_register_custom(sorcery, SIP_SORCERY_AUTH_TYPE, "auth_type",
 			"userpass", auth_type_handler, auth_type_to_str, NULL, 0, 0);
 
-	ast_sip_register_endpoint_formatter(&endpoint_auth_formatter);
+	internal_sip_register_endpoint_formatter(&endpoint_auth_formatter);
 
 	cli_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
 	if (!cli_formatter) {
@@ -337,6 +337,7 @@
 {
 	ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
 	ast_sip_unregister_cli_formatter(cli_formatter);
-
-	return 0;
-}
+	internal_sip_unregister_endpoint_formatter(&endpoint_auth_formatter);
+
+	return 0;
+}

Modified: branches/13/res/res_pjsip/config_transport.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/config_transport.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/config_transport.c (original)
+++ branches/13/res/res_pjsip/config_transport.c Wed Jan 14 17:14:47 2015
@@ -760,7 +760,7 @@
 	ast_sorcery_object_field_register(sorcery, "transport", "cos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, cos));
 	ast_sorcery_object_field_register(sorcery, "transport", "websocket_write_timeout", AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE, FLDSET(struct ast_sip_transport, write_timeout), 1, INT_MAX);
 
-	ast_sip_register_endpoint_formatter(&endpoint_transport_formatter);
+	internal_sip_register_endpoint_formatter(&endpoint_transport_formatter);
 
 	cli_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
 	if (!cli_formatter) {
@@ -786,5 +786,7 @@
 	ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
 	ast_sip_unregister_cli_formatter(cli_formatter);
 
-	return 0;
-}
+	internal_sip_unregister_endpoint_formatter(&endpoint_transport_formatter);
+
+	return 0;
+}

Modified: branches/13/res/res_pjsip/include/res_pjsip_private.h
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/include/res_pjsip_private.h?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/include/res_pjsip_private.h (original)
+++ branches/13/res/res_pjsip/include/res_pjsip_private.h Wed Jan 14 17:14:47 2015
@@ -57,7 +57,15 @@
  * \retval 0 Success
  * \retval non-zero Failure
  */
-int ast_sip_initialize_outbound_authentication(void);
+int internal_sip_initialize_outbound_authentication(void);
+
+/*!
+ * \brief Destroy outbound authentication support
+ *
+ * \retval 0 Success
+ * \retval non-zero Failure
+ */
+void internal_sip_destroy_outbound_authentication(void);
 
 /*!
  * \brief Initialize system configuration
@@ -112,4 +120,24 @@
 int ast_sip_initialize_cli(void);
 void ast_sip_destroy_cli(void);
 
+/*!
+ * \internal \brief Used by res_pjsip.so to register a service without adding a self reference
+ */
+int internal_sip_register_service(pjsip_module *module);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to unregister a service without removing a self reference
+ */
+int internal_sip_unregister_service(pjsip_module *module);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to register an endpoint formatter without adding a self reference
+ */
+void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to unregister a endpoint formatter without removing a self reference
+ */
+int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
+
 #endif /* RES_PJSIP_PRIVATE_H_ */

Modified: branches/13/res/res_pjsip/location.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/location.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/location.c (original)
+++ branches/13/res/res_pjsip/location.c Wed Jan 14 17:14:47 2015
@@ -870,7 +870,7 @@
 	ast_sorcery_object_field_register(sorcery, "aor", "outbound_proxy", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_aor, outbound_proxy));
 	ast_sorcery_object_field_register(sorcery, "aor", "support_path", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_aor, support_path));
 
-	ast_sip_register_endpoint_formatter(&endpoint_aor_formatter);
+	internal_sip_register_endpoint_formatter(&endpoint_aor_formatter);
 
 	contact_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
 	if (!contact_formatter) {
@@ -911,6 +911,8 @@
 	ast_sip_unregister_cli_formatter(contact_formatter);
 	ast_sip_unregister_cli_formatter(aor_formatter);
 
-	return 0;
-}
-
+	internal_sip_unregister_endpoint_formatter(&endpoint_aor_formatter);
+
+	return 0;
+}
+

Modified: branches/13/res/res_pjsip/pjsip_configuration.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_configuration.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/pjsip_configuration.c (original)
+++ branches/13/res/res_pjsip/pjsip_configuration.c Wed Jan 14 17:14:47 2015
@@ -1670,7 +1670,9 @@
 		return -1;
 	}
 
-	ast_sorcery_internal_object_register(sip_sorcery, "nat_hook", sip_nat_hook_alloc, NULL, NULL);
+	if (ast_sorcery_internal_object_register(sip_sorcery, "nat_hook", sip_nat_hook_alloc, NULL, NULL)) {
+		ast_log(LOG_ERROR, "Failed to register nat_hook\n");
+	}
 
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "type", "", OPT_NOOP_T, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "context", "default", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, context));
@@ -1848,6 +1850,7 @@
 	ast_sip_unregister_cli_formatter(endpoint_formatter);
 	ast_sip_unregister_cli_formatter(channel_formatter);
 	ast_sorcery_unref(sip_sorcery);
+	ao2_cleanup(persistent_endpoints);
 }
 
 int ast_res_pjsip_reload_configuration(void)

Modified: branches/13/res/res_pjsip/pjsip_distributor.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_distributor.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/pjsip_distributor.c (original)
+++ branches/13/res/res_pjsip/pjsip_distributor.c Wed Jan 14 17:14:47 2015
@@ -21,6 +21,7 @@
 #include <pjsip.h>
 
 #include "asterisk/res_pjsip.h"
+#include "include/res_pjsip_private.h"
 
 static int distribute(void *data);
 static pj_bool_t distributor(pjsip_rx_data *rdata);
@@ -222,7 +223,7 @@
 	return artificial_auth;
 }
 
-static struct ast_sip_endpoint *artificial_endpoint;
+static struct ast_sip_endpoint *artificial_endpoint = NULL;
 
 static int create_artificial_endpoint(void)
 {
@@ -236,7 +237,7 @@
 	 * the proper size of the vector is returned. This value is
 	 * not actually used anywhere
 	 */
-	AST_VECTOR_APPEND(&artificial_endpoint->inbound_auths, "artificial-auth");
+	AST_VECTOR_APPEND(&artificial_endpoint->inbound_auths, ast_strdup("artificial-auth"));
 	return 0;
 }
 
@@ -373,13 +374,13 @@
 		return -1;
 	}
 
-	if (ast_sip_register_service(&distributor_mod)) {
-		return -1;
-	}
-	if (ast_sip_register_service(&endpoint_mod)) {
-		return -1;
-	}
-	if (ast_sip_register_service(&auth_mod)) {
+	if (internal_sip_register_service(&distributor_mod)) {
+		return -1;
+	}
+	if (internal_sip_register_service(&endpoint_mod)) {
+		return -1;
+	}
+	if (internal_sip_register_service(&auth_mod)) {
 		return -1;
 	}
 
@@ -388,9 +389,9 @@
 
 void ast_sip_destroy_distributor(void)
 {
-	ast_sip_unregister_service(&distributor_mod);
-	ast_sip_unregister_service(&endpoint_mod);
-	ast_sip_unregister_service(&auth_mod);
+	internal_sip_unregister_service(&distributor_mod);
+	internal_sip_unregister_service(&endpoint_mod);
+	internal_sip_unregister_service(&auth_mod);
 
 	ao2_cleanup(artificial_auth);
 	ao2_cleanup(artificial_endpoint);

Modified: branches/13/res/res_pjsip/pjsip_global_headers.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_global_headers.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/pjsip_global_headers.c (original)
+++ branches/13/res/res_pjsip/pjsip_global_headers.c Wed Jan 14 17:14:47 2015
@@ -23,6 +23,7 @@
 
 #include "asterisk/res_pjsip.h"
 #include "asterisk/linkedlists.h"
+#include "include/res_pjsip_private.h"
 
 static pj_status_t add_request_headers(pjsip_tx_data *tdata);
 static pj_status_t add_response_headers(pjsip_tx_data *tdata);
@@ -151,7 +152,7 @@
 	AST_RWLIST_HEAD_INIT(&request_headers);
 	AST_RWLIST_HEAD_INIT(&response_headers);
 
-	ast_sip_register_service(&global_header_mod);
+	internal_sip_register_service(&global_header_mod);
 }
 
 static void destroy_headers(struct header_list *headers)
@@ -168,4 +169,6 @@
 {
 	destroy_headers(&request_headers);
 	destroy_headers(&response_headers);
+
+	internal_sip_unregister_service(&global_header_mod);
 }

Modified: branches/13/res/res_pjsip/pjsip_options.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_options.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/pjsip_options.c (original)
+++ branches/13/res/res_pjsip/pjsip_options.c Wed Jan 14 17:14:47 2015
@@ -1123,7 +1123,7 @@
 		return -1;
 	}
 
-	ast_sip_register_endpoint_formatter(&contact_status_formatter);
+	internal_sip_register_endpoint_formatter(&contact_status_formatter);
 	ast_manager_register2("PJSIPQualify", EVENT_FLAG_SYSTEM | EVENT_FLAG_REPORTING, ami_sip_qualify, NULL, NULL, NULL);
 	ast_cli_register_multiple(cli_options, ARRAY_LEN(cli_options));
 
@@ -1136,7 +1136,7 @@
 {
 	ast_cli_unregister_multiple(cli_options, ARRAY_LEN(cli_options));
 	ast_manager_unregister("PJSIPQualify");
-	ast_sip_unregister_endpoint_formatter(&contact_status_formatter);
+	internal_sip_unregister_endpoint_formatter(&contact_status_formatter);
 
 	pjsip_endpt_unregister_module(ast_sip_get_pjsip_endpoint(), &options_module);
 	ao2_cleanup(sched_qualifies);

Modified: branches/13/res/res_pjsip/pjsip_outbound_auth.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_outbound_auth.c?view=diff&rev=430628&r1=430627&r2=430628
==============================================================================
--- branches/13/res/res_pjsip/pjsip_outbound_auth.c (original)
+++ branches/13/res/res_pjsip/pjsip_outbound_auth.c Wed Jan 14 17:14:47 2015
@@ -91,6 +91,11 @@
 	return 0;
 }
 
-int ast_sip_initialize_outbound_authentication(void) {
-	return ast_sip_register_service(&outbound_auth_mod);
+int internal_sip_initialize_outbound_authentication(void) {
+	return internal_sip_register_service(&outbound_auth_mod);
 }
+
+
+void internal_sip_destroy_outbound_authentication(void) {
+	internal_sip_unregister_service(&outbound_auth_mod);
+}




More information about the svn-commits mailing list