[Asterisk-code-review] Fix shutdown crash caused by modules being left open. (asterisk[master])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/4220 )

Change subject: Fix shutdown crash caused by modules being left open.
......................................................................


Fix shutdown crash caused by modules being left open.

It is only safe to run ast_register_cleanup callbacks when all modules
have been unloaded.  Previously these callbacks were run during graceful
shutdown, making it possible to crash during shutdown.

ASTERISK-26513 #close

Change-Id: Ibfa635bb688d1227ec54aa211d90d6bd45052e21
---
M include/asterisk/_private.h
M include/asterisk/module.h
M main/asterisk.c
M main/loader.c
4 files changed, 8 insertions(+), 10 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index 36b316f..b3c2b20 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -16,6 +16,7 @@
 #define _ASTERISK__PRIVATE_H
 
 int load_modules(unsigned int);		/*!< Provided by loader.c */
+int modules_shutdown(void);		/*!< Provided by loader.c */
 int load_pbx(void);			/*!< Provided by pbx.c */
 int load_pbx_builtins(void);	/*!< Provided by pbx_builtins.c */
 int load_pbx_functions_cli(void);	/*!< Provided by pbx_functions.c */
diff --git a/include/asterisk/module.h b/include/asterisk/module.h
index d5616e9..a80c7f8 100644
--- a/include/asterisk/module.h
+++ b/include/asterisk/module.h
@@ -228,13 +228,6 @@
  */
 int ast_loader_unregister(int (*updater)(void));
 
-/*!
- * \brief Run the unload() callback for all loaded modules
- *
- * This function should be called when Asterisk is shutting down gracefully.
- */
-void ast_module_shutdown(void);
-
 /*! 
  * \brief Match modules names for the Asterisk cli.
  * \param line Unused by this function, but this should be the line we are
diff --git a/main/asterisk.c b/main/asterisk.c
index d4b39a2..56fc107 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -2010,8 +2010,9 @@
 	struct ast_json *json_object = NULL;
 	int run_cleanups = niceness >= SHUTDOWN_NICE;
 
-	if (run_cleanups) {
-		ast_module_shutdown();
+	if (run_cleanups && modules_shutdown()) {
+		ast_verb(0, "Some modules could not be unloaded, switching to fast shutdown\n");
+		run_cleanups = 0;
 	}
 
 	if (!restart) {
diff --git a/main/loader.c b/main/loader.c
index 6617783..dacfce1 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -607,7 +607,7 @@
 
 #endif
 
-void ast_module_shutdown(void)
+int modules_shutdown(void)
 {
 	struct ast_module *mod;
 	int somethingchanged = 1, final = 0;
@@ -655,7 +655,10 @@
 		}
 	} while (somethingchanged && !final);
 
+	final = AST_DLLIST_EMPTY(&module_list);
 	AST_DLLIST_UNLOCK(&module_list);
+
+	return !final;
 }
 
 int ast_unload_resource(const char *resource_name, enum ast_module_unload_mode force)

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfa635bb688d1227ec54aa211d90d6bd45052e21
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>