[Asterisk-code-review] loader: Correct overly strict startup checks. (asterisk[master])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/8048 )

Change subject: loader: Correct overly strict startup checks.
......................................................................

loader: Correct overly strict startup checks.

The code which handled loading modules had too many situations which
would result in halting Asterisk startup.  Treat most errors as declines
instead of failures.  The exception is when the module load function
returns AST_MODULE_LOAD_FAILURE or an invalid code.

Clear the missingdeps vector when appropriate to ensure the next loop
starts clean.

ASTERISK-27620

Change-Id: I45547d9641fd45bd86d80250224417625631ad84
---
M main/loader.c
1 file changed, 19 insertions(+), 13 deletions(-)

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



diff --git a/main/loader.c b/main/loader.c
index da508f3..159014e 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -1394,7 +1394,7 @@
 		}
 		AST_VECTOR_FREE(&missing);
 
-		return AST_MODULE_LOAD_FAILURE;
+		return AST_MODULE_LOAD_DECLINE;
 	}
 
 	if (!ast_fully_booted) {
@@ -1580,6 +1580,7 @@
 		struct ast_module *mod = AST_VECTOR_REMOVE(resources, 0, 1);
 		enum ast_module_load_result lres;
 
+retry_load:
 		lres = start_resource_attempt(mod, mod_count);
 		if (lres == AST_MODULE_LOAD_SUCCESS) {
 			/* No missing dependencies, successful. */
@@ -1598,13 +1599,18 @@
 
 		res = module_deps_missing_recursive(mod, &missingdeps);
 		if (res) {
-			break;
+			AST_VECTOR_RESET(&missingdeps, AST_VECTOR_ELEM_CLEANUP_NOOP);
+			ast_log(LOG_ERROR, "Failed to resolve dependencies for %s\n", ast_module_name(mod));
+			mod->flags.declined = 1;
+
+			continue;
 		}
 
 		if (!AST_VECTOR_SIZE(&missingdeps)) {
-			ast_log(LOG_WARNING, "%s isn't missing any dependencies but still didn't start\n",
-				ast_module_name(mod));
-			/* Dependencies were met but the module failed to start. */
+			ast_log(LOG_WARNING, "%s load function returned an invalid result. "
+				"This is a bug in the module.\n", ast_module_name(mod));
+			/* Dependencies were met but the module failed to start and the result
+			 * code was not AST_MODULE_LOAD_FAILURE or AST_MODULE_LOAD_DECLINE. */
 			res = -1;
 			break;
 		}
@@ -1637,17 +1643,17 @@
 		}
 
 		if (AST_VECTOR_SIZE(&missingdeps)) {
-			ast_log(LOG_ERROR, "Failed to load %s due to unfilled dependencies.\n",
+			ast_log(LOG_WARNING, "Failed to load %s due to unfilled dependencies.\n",
 				ast_module_name(mod));
-			res = -1;
-			break;
+			mod->flags.declined = 1;
+			AST_VECTOR_RESET(&missingdeps, AST_VECTOR_ELEM_CLEANUP_NOOP);
+
+			continue;
 		}
 
-		res = start_resource_attempt(mod, mod_count);
-		if (res) {
-			ast_log(LOG_ERROR, "Failed to load %s: %d\n", ast_module_name(mod), res);
-			break;
-		}
+		/* If we're here it means that we started with missingdeps and they're all loaded
+		 * now.  It's impossible to reach this point a second time for the same module. */
+		goto retry_load;
 	}
 
 	AST_VECTOR_FREE(&missingdeps);

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I45547d9641fd45bd86d80250224417625631ad84
Gerrit-Change-Number: 8048
Gerrit-PatchSet: 1
Gerrit-Owner: 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>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180129/e1dd5e3f/attachment.html>