[Asterisk-code-review] core: Remove non-critical cleanup from startup aborts. (asterisk[13])

Jenkins2 asteriskteam at digium.com
Wed Mar 14 06:40:38 CDT 2018


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8498 )

Change subject: core: Remove non-critical cleanup from startup aborts.
......................................................................

core: Remove non-critical cleanup from startup aborts.

When built-in components of Asterisk fail to start they cause the
Asterisk startup to abort.  In these cases only the most critical
cleanup should be performed - closing databases and terminating
proceses.  These cleanups are registered using ast_register_atexit, all
other cleanups should not be run during startup abort.

The main reason for this change is that these cleanup procedures are
untestable from the partially initialized states, if they fail it could
prevent us from ever running the critical cleanup with ast_run_atexits.

Change-Id: Iecc2df98008b21509925ff16740bd5fa29527db3
---
M main/cel.c
M main/core_local.c
M main/devicestate.c
M main/dsp.c
M main/features.c
M main/features_config.c
M main/indications.c
M main/pbx_builtins.c
M main/sorcery.c
9 files changed, 13 insertions(+), 46 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/main/cel.c b/main/cel.c
index 31cd045..45726e0 100644
--- a/main/cel.c
+++ b/main/cel.c
@@ -1565,7 +1565,6 @@
 	ao2_global_obj_replace_unref(cel_linkedids, container);
 	ao2_cleanup(container);
 	if (!container) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
@@ -1574,17 +1573,14 @@
 	ao2_global_obj_replace_unref(cel_dialstatus_store, container);
 	ao2_cleanup(container);
 	if (!container) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
 	if (STASIS_MESSAGE_TYPE_INIT(cel_generic_type)) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
 	if (ast_cli_register(&cli_status)) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
@@ -1592,12 +1588,10 @@
 	ao2_global_obj_replace_unref(cel_backends, container);
 	ao2_cleanup(container);
 	if (!container) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
 	if (aco_info_init(&cel_cfg_info)) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
@@ -1610,7 +1604,6 @@
 		struct cel_config *cel_cfg = cel_config_alloc();
 
 		if (!cel_cfg) {
-			cel_engine_cleanup();
 			return -1;
 		}
 
@@ -1623,12 +1616,10 @@
 	}
 
 	if (create_subscriptions()) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
 	if (ast_cel_check_enabled() && create_routes()) {
-		cel_engine_cleanup();
 		return -1;
 	}
 
diff --git a/main/core_local.c b/main/core_local.c
index a5918f5..12e41f9 100644
--- a/main/core_local.c
+++ b/main/core_local.c
@@ -1074,7 +1074,6 @@
 
 int ast_local_init(void)
 {
-
 	if (STASIS_MESSAGE_TYPE_INIT(ast_local_optimization_begin_type)) {
 		return -1;
 	}
@@ -1094,17 +1093,13 @@
 
 	locals = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_MUTEX, 0, NULL, locals_cmp_cb);
 	if (!locals) {
-		ao2_cleanup(local_tech.capabilities);
-		local_tech.capabilities = NULL;
 		return -1;
 	}
 
 	/* Make sure we can register our channel type */
 	if (ast_channel_register(&local_tech)) {
 		ast_log(LOG_ERROR, "Unable to register channel class 'Local'\n");
-		ao2_ref(locals, -1);
-		ao2_cleanup(local_tech.capabilities);
-		local_tech.capabilities = NULL;
+
 		return -1;
 	}
 	ast_cli_register_multiple(cli_local, ARRAY_LEN(cli_local));
diff --git a/main/devicestate.c b/main/devicestate.c
index faba144..4bc0bed 100644
--- a/main/devicestate.c
+++ b/main/devicestate.c
@@ -913,24 +913,20 @@
 	}
 	device_state_topic_all = stasis_topic_create("ast_device_state_topic");
 	if (!device_state_topic_all) {
-		devstate_cleanup();
 		return -1;
 	}
 	device_state_topic_pool = stasis_topic_pool_create(ast_device_state_topic_all());
 	if (!device_state_topic_pool) {
-		devstate_cleanup();
 		return -1;
 	}
 	device_state_cache = stasis_cache_create_full(device_state_get_id,
 		device_state_aggregate_calc, device_state_aggregate_publish);
 	if (!device_state_cache) {
-		devstate_cleanup();
 		return -1;
 	}
 	device_state_topic_cached = stasis_caching_topic_create(ast_device_state_topic_all(),
 		device_state_cache);
 	if (!device_state_topic_cached) {
-		devstate_cleanup();
 		return -1;
 	}
 
@@ -938,7 +934,6 @@
 		devstate_change_cb, NULL);
 	if (!devstate_message_sub) {
 		ast_log(LOG_ERROR, "Failed to create subscription creating uncached device state aggregate events.\n");
-		devstate_cleanup();
 		return -1;
 	}
 
diff --git a/main/dsp.c b/main/dsp.c
index 0609256..e3e1924 100644
--- a/main/dsp.c
+++ b/main/dsp.c
@@ -2404,17 +2404,18 @@
 
 int ast_dsp_init(void)
 {
-	int res = _dsp_init(0);
+	if (_dsp_init(0)) {
+		return -1;
+	}
 
 #ifdef TEST_FRAMEWORK
-	if (!res) {
-		AST_TEST_REGISTER(test_dsp_fax_detect);
-		AST_TEST_REGISTER(test_dsp_dtmf_detect);
+	AST_TEST_REGISTER(test_dsp_fax_detect);
+	AST_TEST_REGISTER(test_dsp_dtmf_detect);
 
-		ast_register_cleanup(test_dsp_shutdown);
-	}
+	ast_register_cleanup(test_dsp_shutdown);
 #endif
-	return res;
+
+	return 0;
 }
 
 int ast_dsp_reload(void)
diff --git a/main/features.c b/main/features.c
index 2ca56bc..21e6c22 100644
--- a/main/features.c
+++ b/main/features.c
@@ -1167,17 +1167,10 @@
 	int res;
 
 	res = ast_features_config_init();
-	if (res) {
-		return res;
-	}
 	res |= ast_register_application2(app_bridge, bridge_exec, NULL, NULL, NULL);
 	res |= ast_manager_register_xml_core("Bridge", EVENT_FLAG_CALL, action_bridge);
 
-	if (res) {
-		features_shutdown();
-	} else {
-		ast_register_cleanup(features_shutdown);
-	}
+	ast_register_cleanup(features_shutdown);
 
 	return res;
 }
diff --git a/main/features_config.c b/main/features_config.c
index e2d4057..3dac827 100644
--- a/main/features_config.c
+++ b/main/features_config.c
@@ -2000,9 +2000,5 @@
 	res |= __ast_custom_function_register(&featuremap_function, NULL);
 	res |= ast_cli_register_multiple(cli_features_config, ARRAY_LEN(cli_features_config));
 
-	if (res) {
-		ast_features_config_shutdown();
-	}
-
 	return res;
 }
diff --git a/main/indications.c b/main/indications.c
index 4888680..8ca1068 100644
--- a/main/indications.c
+++ b/main/indications.c
@@ -1173,13 +1173,13 @@
 /*! \brief Load indications module */
 int ast_indications_init(void)
 {
-	if (!(ast_tone_zones = ao2_container_alloc(NUM_TONE_ZONE_BUCKETS,
-			ast_tone_zone_hash, ast_tone_zone_cmp))) {
+	ast_tone_zones = ao2_container_alloc(NUM_TONE_ZONE_BUCKETS,
+			ast_tone_zone_hash, ast_tone_zone_cmp);
+	if (!ast_tone_zones) {
 		return -1;
 	}
 
 	if (load_indications(0)) {
-		indications_shutdown();
 		return -1;
 	}
 
diff --git a/main/pbx_builtins.c b/main/pbx_builtins.c
index 605e0c9..44418b8 100644
--- a/main/pbx_builtins.c
+++ b/main/pbx_builtins.c
@@ -1433,7 +1433,6 @@
 	for (x = 0; x < ARRAY_LEN(builtins); x++) {
 		if (ast_register_application2(builtins[x].name, builtins[x].execute, NULL, NULL, NULL)) {
 			ast_log(LOG_ERROR, "Unable to register builtin application '%s'\n", builtins[x].name);
-			unload_pbx_builtins();
 			return -1;
 		}
 	}
diff --git a/main/sorcery.c b/main/sorcery.c
index c20854f..2418ce3 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -383,20 +383,17 @@
 	wizards = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0, WIZARD_BUCKETS,
 		ast_sorcery_internal_wizard_hash_fn, NULL, ast_sorcery_internal_wizard_cmp_fn);
 	if (!wizards) {
-		sorcery_cleanup();
 		return -1;
 	}
 
 	observers = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_RWLOCK, 0, NULL, NULL);
 	if (!observers) {
-		sorcery_cleanup();
 		return -1;
 	}
 
 	instances = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_RWLOCK, 0, INSTANCE_BUCKETS,
 		ast_sorcery_hash_fn, NULL, ast_sorcery_cmp_fn);
 	if (!instances) {
-		sorcery_cleanup();
 		return -1;
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Iecc2df98008b21509925ff16740bd5fa29527db3
Gerrit-Change-Number: 8498
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180314/2d843858/attachment.html>


More information about the asterisk-code-review mailing list