[Asterisk-code-review] res pjsip pubsub: Prevent unload except during shutdown. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/6833 )

Change subject: res_pjsip_pubsub: Prevent unload except during shutdown.
......................................................................

res_pjsip_pubsub: Prevent unload except during shutdown.

Prevent unload of the module as certain pjsip initialization functions
cannot be reversed.  This required a reorder of the module_load so that
the non-reversable pjsip functions are not called until all potential
errors have been ruled out.

ASTERISK-24483

Change-Id: Iee900f20bdd6ee1bfe23efdec0d87765eadce8a7
---
M res/res_pjsip_pubsub.c
1 file changed, 21 insertions(+), 15 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Joshua Colp: Approved for Submit



diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index bcf8677..a1bfbb7 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -5355,25 +5355,13 @@
 
 	sorcery = ast_sip_get_sorcery();
 
-	pjsip_evsub_init_module(ast_sip_get_pjsip_endpoint());
-
 	if (!(sched = ast_sched_context_create())) {
 		ast_log(LOG_ERROR, "Could not create scheduler for publication expiration\n");
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	pjsip_media_type_init2(&rlmi_media_type, "application", "rlmi+xml");
-
 	if (ast_sched_start_thread(sched)) {
 		ast_log(LOG_ERROR, "Could not start scheduler thread for publication expiration\n");
-		ast_sched_context_destroy(sched);
-		return AST_MODULE_LOAD_DECLINE;
-	}
-
-	pjsip_endpt_add_capability(ast_sip_get_pjsip_endpoint(), NULL, PJSIP_H_ALLOW, NULL, 1, &str_PUBLISH);
-
-	if (ast_sip_register_service(&pubsub_module)) {
-		ast_log(LOG_ERROR, "Could not register pubsub service\n");
 		ast_sched_context_destroy(sched);
 		return AST_MODULE_LOAD_DECLINE;
 	}
@@ -5383,7 +5371,6 @@
 	if (ast_sorcery_object_register(sorcery, "subscription_persistence", subscription_persistence_alloc,
 		NULL, NULL)) {
 		ast_log(LOG_ERROR, "Could not register subscription persistence object support\n");
-		ast_sip_unregister_service(&pubsub_module);
 		ast_sched_context_destroy(sched);
 		return AST_MODULE_LOAD_DECLINE;
 	}
@@ -5411,7 +5398,6 @@
 		CHARFLDSET(struct subscription_persistence, contact_uri));
 
 	if (apply_list_configuration(sorcery)) {
-		ast_sip_unregister_service(&pubsub_module);
 		ast_sched_context_destroy(sched);
 		return AST_MODULE_LOAD_DECLINE;
 	}
@@ -5420,7 +5406,6 @@
 	if (ast_sorcery_object_register(sorcery, "inbound-publication", publication_resource_alloc,
 		NULL, NULL)) {
 		ast_log(LOG_ERROR, "Could not register subscription persistence object support\n");
-		ast_sip_unregister_service(&pubsub_module);
 		ast_sched_context_destroy(sched);
 		return AST_MODULE_LOAD_DECLINE;
 	}
@@ -5430,6 +5415,27 @@
 	ast_sorcery_object_fields_register(sorcery, "inbound-publication", "^event_", resource_event_handler, NULL);
 	ast_sorcery_reload_object(sorcery, "inbound-publication");
 
+	if (ast_sip_register_service(&pubsub_module)) {
+		ast_log(LOG_ERROR, "Could not register pubsub service\n");
+		ast_sched_context_destroy(sched);
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	if (pjsip_evsub_init_module(ast_sip_get_pjsip_endpoint()) != PJ_SUCCESS) {
+		ast_log(LOG_ERROR, "Could not initialize pjsip evsub module.\n");
+		ast_sip_unregister_service(&pubsub_module);
+		ast_sched_context_destroy(sched);
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	/* Once pjsip_evsub_init_module succeeds we cannot unload.
+	 * Keep all module_load errors above this point. */
+	ast_module_shutdown_ref(ast_module_info->self);
+
+	pjsip_media_type_init2(&rlmi_media_type, "application", "rlmi+xml");
+
+	pjsip_endpt_add_capability(ast_sip_get_pjsip_endpoint(), NULL, PJSIP_H_ALLOW, NULL, 1, &str_PUBLISH);
+
 	if (ast_test_flag(&ast_options, AST_OPT_FLAG_FULLY_BOOTED)) {
 		ast_sip_push_task(NULL, subscription_persistence_load, NULL);
 	} else {

-- 
To view, visit https://gerrit.asterisk.org/6833
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iee900f20bdd6ee1bfe23efdec0d87765eadce8a7
Gerrit-Change-Number: 6833
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171020/fc4a62ad/attachment.html>