[Asterisk-code-review] chan sip: Reorder unload module to deal with stuck TCP threads. (asterisk[master])

[Joshua Colp]

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

Change subject: chan_sip: Reorder unload_module to deal with stuck TCP threads.
......................................................................


chan_sip: Reorder unload_module to deal with stuck TCP threads.

In some situations TCP threads may become frozen.  This creates the
possibility that Asterisk could segfault if they become unfrozen after
chan_sip has been dlclose'd.  This reorders the unload_module process to
allow abort if threads do not exit within 5 seconds.

High level order as follows:
1) Unregister from the core to stop new requests.
2) Signal threads to stop
3) Clear config based tables (but do not free the table itself).
4) Verify that threads have shutdown, cancel unload if not.
5) Clean all remaining resources.

ASTERISK-26586

Change-Id: Ie23692041d838fbd35ece61868f4c640960ff882
---
M channels/chan_sip.c
1 file changed, 30 insertions(+), 30 deletions(-)

Approvals:
  Michael Kuron: Looks good to me, but someone else must approve
  Matthew Fredrickson: Looks good to me, approved
  Joshua Colp: Verified



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index aa0d6a5..af17a38 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -1780,7 +1780,7 @@
 {
 	int i;
 	for (i = 0; i < ARRAY_LEN(event_state_compositors); i++) {
-		ao2_cleanup(event_state_compositors[i].compositor);
+		ao2_replace(event_state_compositors[i].compositor, NULL);
 	}
 }
 
@@ -35365,6 +35365,8 @@
 	struct ao2_iterator i;
 	struct timeval start;
 
+	ast_sched_dump(sched);
+
 	ast_sip_api_provider_unregister();
 
 	if (sip_cfg.websocket_enabled) {
@@ -35374,12 +35376,11 @@
 	network_change_stasis_unsubscribe();
 	acl_change_event_stasis_unsubscribe();
 
-	ast_sched_dump(sched);
-
 	/* First, take us out of the channel type list */
 	ast_channel_unregister(&sip_tech);
-
 	ast_msg_tech_unregister(&sip_msg_tech);
+	ast_cc_monitor_unregister(&sip_cc_monitor_callbacks);
+	ast_cc_agent_unregister(&sip_cc_agent_callbacks);
 
 	/* Unregister dial plan functions */
 	ast_custom_function_unregister(&sippeer_function);
@@ -35443,8 +35444,6 @@
 	}
 	ao2_iterator_destroy(&i);
 
-	unlink_all_peers_from_tables();
-
 	ast_mutex_lock(&monlock);
 	if (monitor_thread && (monitor_thread != AST_PTHREADT_STOP) && (monitor_thread != AST_PTHREADT_NULL)) {
 		pthread_t th = monitor_thread;
@@ -35458,7 +35457,12 @@
 		ast_mutex_unlock(&monlock);
 	}
 
+	/* Clear containers */
+	unlink_all_peers_from_tables();
 	cleanup_all_regs();
+	sip_epa_unregister_all();
+	destroy_escs();
+	clear_sip_domains();
 
 	{
 		struct ao2_iterator iter;
@@ -35487,27 +35491,6 @@
 	 */
 	ast_sched_runq(sched);
 
-	/* Free memory for local network address mask */
-	ast_free_ha(localaddr);
-
-	ast_mutex_lock(&authl_lock);
-	if (authl) {
-		ao2_t_cleanup(authl, "Removing global authentication");
-		authl = NULL;
-	}
-	ast_mutex_unlock(&authl_lock);
-
-	sip_epa_unregister_all();
-	destroy_escs();
-
-	ast_free(default_tls_cfg.certfile);
-	ast_free(default_tls_cfg.pvtfile);
-	ast_free(default_tls_cfg.cipher);
-	ast_free(default_tls_cfg.cafile);
-	ast_free(default_tls_cfg.capath);
-
-	ast_rtp_dtls_cfg_free(&default_dtls_cfg);
-
 	/*
 	 * Wait awhile for the TCP/TLS thread container to become empty.
 	 *
@@ -35521,7 +35504,27 @@
 	}
 	if (ao2_container_count(threadt)) {
 		ast_debug(2, "TCP/TLS thread container did not become empty :(\n");
+
+		return -1;
 	}
+
+	/* Free memory for local network address mask */
+	ast_free_ha(localaddr);
+
+	ast_mutex_lock(&authl_lock);
+	if (authl) {
+		ao2_t_cleanup(authl, "Removing global authentication");
+		authl = NULL;
+	}
+	ast_mutex_unlock(&authl_lock);
+
+	ast_free(default_tls_cfg.certfile);
+	ast_free(default_tls_cfg.pvtfile);
+	ast_free(default_tls_cfg.cipher);
+	ast_free(default_tls_cfg.cafile);
+	ast_free(default_tls_cfg.capath);
+
+	ast_rtp_dtls_cfg_free(&default_dtls_cfg);
 
 	ao2_cleanup(registry_list);
 	ao2_cleanup(subscription_mwi_list);
@@ -35536,7 +35539,6 @@
 	ao2_t_cleanup(threadt, "unref the thread table");
 	ao2_t_cleanup(sip_monitor_instances, "unref the sip_monitor_instances table");
 
-	clear_sip_domains();
 	sip_cfg.contact_acl = ast_free_acl_list(sip_cfg.contact_acl);
 	if (sipsock_read_id) {
 		ast_io_remove(io, sipsock_read_id);
@@ -35549,8 +35551,6 @@
 	ast_context_destroy_by_name(used_context, "SIP");
 	ast_unload_realtime("sipregs");
 	ast_unload_realtime("sippeers");
-	ast_cc_monitor_unregister(&sip_cc_monitor_callbacks);
-	ast_cc_agent_unregister(&sip_cc_agent_callbacks);
 
 	sip_reqresp_parser_exit();
 	sip_unregister_tests();

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie23692041d838fbd35ece61868f4c640960ff882
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Michael Kuron <m.kuron at gmx.de>