[Asterisk-code-review] core: Remove non-critical cleanup from startup aborts. (asterisk[15])
Jenkins2
asteriskteam at digium.com
Wed Mar 14 09:37:46 CDT 2018
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8499 )
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.
Create separate initialization for dns_core.c to be run unconditionally
during startup instead of being initialized by the first dns resolver to
be registered. This ensures that 'sched' is initialized before it can be
potentially used.
Replace ast_register_atexit with ast_register_cleanup in media_cache.c.
There is no reason for this cleanup to happen unconditionally.
Change-Id: Iecc2df98008b21509925ff16740bd5fa29527db3
---
M include/asterisk/_private.h
M main/asterisk.c
M main/cel.c
M main/core_local.c
M main/devicestate.c
M main/dns_core.c
M main/dns_system_resolver.c
M main/dsp.c
M main/features.c
M main/features_config.c
M main/indications.c
M main/media_cache.c
M main/pbx_builtins.c
M main/sorcery.c
14 files changed, 34 insertions(+), 71 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/include/asterisk/_private.h b/include/asterisk/_private.h
index e989b16..b9f552d 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -59,6 +59,7 @@
int ast_msg_init(void); /*!< Provided by message.c */
void ast_msg_shutdown(void); /*!< Provided by message.c */
int aco_init(void); /*!< Provided by config_options.c */
+int dns_core_init(void); /*!< Provided by dns_core.c */
/*!
* \brief Initialize the bridging system.
diff --git a/main/asterisk.c b/main/asterisk.c
index e44c896..f5d1c21 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -4573,6 +4573,7 @@
check_init(ast_parking_stasis_init(), "Parking Core");
check_init(ast_device_state_engine_init(), "Device State Engine");
check_init(ast_presence_state_engine_init(), "Presence State Engine");
+ check_init(dns_core_init(), "DNS Resolver Core");
check_init(ast_dns_system_resolver_init(), "Default DNS resolver");
check_init(load_modules(1), "Module Preload");
check_init(ast_features_init(), "Call Features");
diff --git a/main/cel.c b/main/cel.c
index e4fae6d..2813e60 100644
--- a/main/cel.c
+++ b/main/cel.c
@@ -1563,7 +1563,6 @@
ao2_global_obj_replace_unref(cel_linkedids, container);
ao2_cleanup(container);
if (!container) {
- cel_engine_cleanup();
return -1;
}
@@ -1572,17 +1571,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;
}
@@ -1590,12 +1586,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;
}
@@ -1608,7 +1602,6 @@
struct cel_config *cel_cfg = cel_config_alloc();
if (!cel_cfg) {
- cel_engine_cleanup();
return -1;
}
@@ -1621,12 +1614,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 23c7cce..c3fa15f 100644
--- a/main/core_local.c
+++ b/main/core_local.c
@@ -1049,7 +1049,6 @@
int ast_local_init(void)
{
-
if (STASIS_MESSAGE_TYPE_INIT(ast_local_optimization_begin_type)) {
return -1;
}
@@ -1069,17 +1068,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 1db9a19..5df3449 100644
--- a/main/devicestate.c
+++ b/main/devicestate.c
@@ -911,24 +911,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;
}
@@ -936,7 +932,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/dns_core.c b/main/dns_core.c
index 3e270af..6f37a5d 100644
--- a/main/dns_core.c
+++ b/main/dns_core.c
@@ -29,6 +29,7 @@
#include "asterisk.h"
+#include "asterisk/_private.h"
#include "asterisk/linkedlists.h"
#include "asterisk/astobj2.h"
#include "asterisk/strings.h"
@@ -537,6 +538,22 @@
}
}
+int dns_core_init(void)
+{
+ sched = ast_sched_context_create();
+ if (!sched) {
+ return -1;
+ }
+
+ if (ast_sched_start_thread(sched)) {
+ return -1;
+ }
+
+ ast_register_cleanup(dns_shutdown);
+
+ return 0;
+}
+
int ast_dns_resolver_register(struct ast_dns_resolver *resolver)
{
struct ast_dns_resolver *iter;
@@ -558,27 +575,6 @@
}
AST_RWLIST_WRLOCK(&resolvers);
-
- /* On the first registration of a resolver start a scheduler for recurring queries */
- if (AST_LIST_EMPTY(&resolvers) && !sched) {
- sched = ast_sched_context_create();
- if (!sched) {
- ast_log(LOG_ERROR, "DNS resolver '%s' could not be registered: Failed to create scheduler for recurring DNS queries\n",
- resolver->name);
- AST_RWLIST_UNLOCK(&resolvers);
- return -1;
- }
-
- if (ast_sched_start_thread(sched)) {
- ast_log(LOG_ERROR, "DNS resolver '%s' could not be registered: Failed to start thread for recurring DNS queries\n",
- resolver->name);
- dns_shutdown();
- AST_RWLIST_UNLOCK(&resolvers);
- return -1;
- }
-
- ast_register_cleanup(dns_shutdown);
- }
AST_LIST_TRAVERSE(&resolvers, iter, next) {
if (!strcmp(iter->name, resolver->name)) {
diff --git a/main/dns_system_resolver.c b/main/dns_system_resolver.c
index 9358577..8cb92c0 100644
--- a/main/dns_system_resolver.c
+++ b/main/dns_system_resolver.c
@@ -255,7 +255,6 @@
/* Return error if the task processor failed to instantiate */
if (!dns_system_resolver_tp) {
- dns_system_resolver_destroy();
return DNS_SYSTEM_RESOLVER_FAILURE;
}
diff --git a/main/dsp.c b/main/dsp.c
index 8b39fe5..66d95ad 100644
--- a/main/dsp.c
+++ b/main/dsp.c
@@ -2402,17 +2402,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 516c64a..35039e0 100644
--- a/main/features.c
+++ b/main/features.c
@@ -1164,17 +1164,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 a773f49..195fbac 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 bde6e01..8971058 100644
--- a/main/indications.c
+++ b/main/indications.c
@@ -1130,13 +1130,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/media_cache.c b/main/media_cache.c
index 90057dc..e93d1a0 100644
--- a/main/media_cache.c
+++ b/main/media_cache.c
@@ -645,7 +645,7 @@
*/
static void media_cache_shutdown(void)
{
- ao2_ref(media_cache, -1);
+ ao2_cleanup(media_cache);
media_cache = NULL;
ast_cli_unregister_multiple(cli_media_cache, ARRAY_LEN(cli_media_cache));
@@ -653,7 +653,7 @@
int ast_media_cache_init(void)
{
- ast_register_atexit(media_cache_shutdown);
+ ast_register_cleanup(media_cache_shutdown);
media_cache = ao2_container_alloc_options(AO2_ALLOC_OPT_LOCK_MUTEX, AO2_BUCKETS,
ast_sorcery_object_id_hash, ast_sorcery_object_id_compare);
@@ -662,7 +662,6 @@
}
if (ast_cli_register_multiple(cli_media_cache, ARRAY_LEN(cli_media_cache))) {
- ao2_ref(media_cache, -1);
return -1;
}
diff --git a/main/pbx_builtins.c b/main/pbx_builtins.c
index 9d43c10..7f76b97 100644
--- a/main/pbx_builtins.c
+++ b/main/pbx_builtins.c
@@ -1509,7 +1509,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 f4c5dce..d95a6e1 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -388,20 +388,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,
sorcery_proxy_hash_fn, NULL, sorcery_proxy_cmp_fn);
if (!instances) {
- sorcery_cleanup();
return -1;
}
--
To view, visit https://gerrit.asterisk.org/8499
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Iecc2df98008b21509925ff16740bd5fa29527db3
Gerrit-Change-Number: 8499
Gerrit-PatchSet: 3
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: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180314/3e4f36be/attachment-0001.html>
More information about the asterisk-code-review
mailing list