[asterisk-commits] res pjsip: Refactor load module/unload module (asterisk[13])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Feb 12 16:58:20 CST 2016
Joshua Colp has submitted this change and it was merged.
Change subject: res_pjsip: Refactor load_module/unload_module
......................................................................
res_pjsip: Refactor load_module/unload_module
load_module was just too hairy with every step having to clean up all
previous steps on failure.
Some of the pjproject init calls have now been moved to a separate
load_pjsip function and the unload_pjsip function was enhanced to clean
up everything if an error happened at any stage of the load process.
In the process, a bunch of missing pj_shutdowns, serializer_pool_shutdowns
and ast_threadpool_shutdowns were also corrected.
Change-Id: I5eec711b437c35b56605ed99537ebbb30463b302
---
M res/res_pjsip.c
M res/res_pjsip/pjsip_configuration.c
2 files changed, 109 insertions(+), 132 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Verified
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index d0d6ba9..d498c8d 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3908,31 +3908,55 @@
return 0;
}
-static int load_module(void)
+static int unload_pjsip(void *data)
{
+ /*
+ * These calls need the pjsip endpoint and serializer to clean up.
+ * If they're not set, then there's nothing to clean up anyway.
+ */
+ if (ast_pjsip_endpoint && serializer_pool[0]) {
+ 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();
+ monitor_thread = NULL;
+ }
+
+ if (memory_pool) {
+ pj_pool_release(memory_pool);
+ memory_pool = NULL;
+ }
+
+ ast_pjsip_endpoint = NULL;
+
+ if (caching_pool.lock) {
+ pj_caching_pool_destroy(&caching_pool);
+ }
+
+ pj_shutdown();
+
+ return 0;
+}
+
+static int load_pjsip(void)
+{
+ pj_status_t status;
+
/* The third parameter is just copied from
* example code from PJLIB. This can be adjusted
* if necessary.
*/
- pj_status_t status;
- struct ast_threadpool_options options;
-
- CHECK_PJPROJECT_MODULE_LOADED();
-
- if (pj_init() != PJ_SUCCESS) {
- return AST_MODULE_LOAD_DECLINE;
- }
-
- if (pjlib_util_init() != PJ_SUCCESS) {
- pj_shutdown();
- return AST_MODULE_LOAD_DECLINE;
- }
-
pj_caching_pool_init(&caching_pool, NULL, 1024 * 1024);
if (pjsip_endpt_create(&caching_pool.factory, "SIP", &ast_pjsip_endpoint) != PJ_SUCCESS) {
ast_log(LOG_ERROR, "Failed to create PJSIP endpoint structure. Aborting load\n");
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
/* PJSIP will automatically try to add a Max-Forwards header. Since we want to control that,
@@ -3943,10 +3967,7 @@
memory_pool = pj_pool_create(&caching_pool.factory, "SIP", 1024, 1024, NULL);
if (!memory_pool) {
ast_log(LOG_ERROR, "Failed to create memory pool for SIP. Aborting load\n");
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
if (!pj_gethostip(pj_AF_INET(), &host_ip_ipv4)) {
@@ -3959,44 +3980,6 @@
ast_verb(3, "Local IPv6 address determined to be: %s\n", host_ip_ipv6_string);
}
- if (ast_sip_initialize_system()) {
- ast_log(LOG_ERROR, "Failed to initialize SIP 'system' configuration section. Aborting load\n");
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
- }
-
- sip_get_threadpool_options(&options);
- options.thread_start = sip_thread_start;
- sip_threadpool = ast_threadpool_create("SIP", NULL, &options);
- if (!sip_threadpool) {
- ast_log(LOG_ERROR, "Failed to create SIP threadpool. Aborting load\n");
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
- }
-
- if (serializer_pool_setup()) {
- ast_log(LOG_ERROR, "Failed to create SIP serializer pool. Aborting load\n");
- ast_threadpool_shutdown(sip_threadpool);
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
- }
-
- ast_sip_initialize_dns();
-
pjsip_tsx_layer_init_module(ast_pjsip_endpoint);
pjsip_ua_init_module(ast_pjsip_endpoint, NULL);
@@ -4005,73 +3988,76 @@
NULL, PJ_THREAD_DEFAULT_STACK_SIZE * 2, 0, &monitor_thread);
if (status != PJ_SUCCESS) {
ast_log(LOG_ERROR, "Failed to start SIP monitor thread. Aborting load\n");
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
+ goto error;
+ }
+
+ return AST_MODULE_LOAD_SUCCESS;
+
+error:
+ unload_pjsip(NULL);
+ return AST_MODULE_LOAD_DECLINE;
+}
+
+static int load_module(void)
+{
+ struct ast_threadpool_options options;
+
+ CHECK_PJPROJECT_MODULE_LOADED();
+
+ /* pjproject and config_system need to be initialized before all else */
+ if (pj_init() != PJ_SUCCESS) {
return AST_MODULE_LOAD_DECLINE;
}
+
+ if (pjlib_util_init() != PJ_SUCCESS) {
+ goto error;
+ }
+
+ if (ast_sip_initialize_system()) {
+ ast_log(LOG_ERROR, "Failed to initialize SIP 'system' configuration section. Aborting load\n");
+ goto error;
+ }
+
+ /* The serializer needs threadpool and threadpool needs pjproject to be initialized so it's next */
+ sip_get_threadpool_options(&options);
+ options.thread_start = sip_thread_start;
+ sip_threadpool = ast_threadpool_create("SIP", NULL, &options);
+ if (!sip_threadpool) {
+ goto error;
+ }
+
+ if (serializer_pool_setup()) {
+ ast_log(LOG_ERROR, "Failed to create SIP serializer pool. Aborting load\n");
+ goto error;
+ }
+
+ /* Now load all the pjproject infrastructure. */
+ if (load_pjsip()) {
+ goto error;
+ }
+
+ ast_sip_initialize_dns();
ast_sip_initialize_global_headers();
if (ast_res_pjsip_initialize_configuration(ast_module_info)) {
ast_log(LOG_ERROR, "Failed to initialize SIP configuration. Aborting load\n");
- ast_sip_destroy_global_headers();
- stop_monitor_thread();
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
if (ast_sip_initialize_distributor()) {
ast_log(LOG_ERROR, "Failed to register distributor module. Aborting load\n");
- ast_res_pjsip_destroy_configuration();
- ast_sip_destroy_global_headers();
- stop_monitor_thread();
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
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();
- ast_sip_destroy_global_headers();
- stop_monitor_thread();
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
if (internal_sip_initialize_outbound_authentication()) {
ast_log(LOG_ERROR, "Failed to initialize outbound authentication. Aborting load\n");
- internal_sip_unregister_service(&supplement_module);
- ast_sip_destroy_distributor();
- ast_res_pjsip_destroy_configuration();
- ast_sip_destroy_global_headers();
- stop_monitor_thread();
- ast_sip_destroy_system();
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- pjsip_endpt_destroy(ast_pjsip_endpoint);
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- return AST_MODULE_LOAD_DECLINE;
+ goto error;
}
ast_res_pjsip_init_options_handling(0);
@@ -4083,6 +4069,14 @@
ast_pjproject_ref();
return AST_MODULE_LOAD_SUCCESS;
+
+error:
+ /* These functions all check for NULLs and are safe to call at any time */
+ unload_pjsip(NULL);
+ serializer_pool_shutdown();
+ ast_threadpool_shutdown(sip_threadpool);
+
+ return AST_MODULE_LOAD_DECLINE;
}
static int reload_module(void)
@@ -4099,33 +4093,11 @@
return 0;
}
-static int unload_pjsip(void *data)
-{
- ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
- 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();
- }
- if (memory_pool) {
- pj_pool_release(memory_pool);
- memory_pool = NULL;
- }
- ast_pjsip_endpoint = NULL;
- pj_caching_pool_destroy(&caching_pool);
- pj_shutdown();
- return 0;
-}
-
static int unload_module(void)
{
AST_TEST_UNREGISTER(xml_sanitization_end_null);
AST_TEST_UNREGISTER(xml_sanitization_exceeds_buffer);
+ ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
/* The thread this is called from cannot call PJSIP/PJLIB functions,
* so we have to push the work to the threadpool to handle
@@ -4135,7 +4107,6 @@
serializer_pool_shutdown();
ast_threadpool_shutdown(sip_threadpool);
- ast_sip_destroy_cli();
ast_pjproject_unref();
return 0;
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index d4473ff..1eed928 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -2026,18 +2026,24 @@
void ast_res_pjsip_destroy_configuration(void)
{
+ if (!sip_sorcery) {
+ return;
+ }
+
ast_sorcery_observer_remove(sip_sorcery, CONTACT_STATUS, &state_contact_status_observer);
ast_sorcery_observer_remove(sip_sorcery, "contact", &state_contact_observer);
ast_sip_destroy_sorcery_global();
ast_sip_destroy_sorcery_location();
ast_sip_destroy_sorcery_auth();
ast_sip_destroy_sorcery_transport();
+ ast_sorcery_unref(sip_sorcery);
+ sip_sorcery = NULL;
ast_manager_unregister(AMI_SHOW_ENDPOINT);
ast_manager_unregister(AMI_SHOW_ENDPOINTS);
ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
ast_sip_unregister_cli_formatter(endpoint_formatter);
ast_sip_unregister_cli_formatter(channel_formatter);
- ast_sorcery_unref(sip_sorcery);
+ ast_sip_destroy_cli();
ao2_cleanup(persistent_endpoints);
}
--
To view, visit https://gerrit.asterisk.org/2224
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5eec711b437c35b56605ed99537ebbb30463b302
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-commits
mailing list