[Asterisk-code-review] res pjsip outbound registration.c: Re-REGISTER on transport ... (asterisk[15])

Richard Mudgett asteriskteam at digium.com
Wed Aug 9 13:06:37 CDT 2017


Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/6202


Change subject: res_pjsip_outbound_registration.c: Re-REGISTER on transport shutdown.
......................................................................

res_pjsip_outbound_registration.c: Re-REGISTER on transport shutdown.

The fix for the issue is broken up into three parts.

This is part three which handles the client side of REGISTER requests.
The registered contact may no longer be valid on the server when the
transport used is reliable and the connection is broken.

* Re-REGISTER our contact if the reliable transport is broken after
registration completes.  We attempt to re-REGISTER immediately to minimize
the time we are unreachable.  Time may have already passed between the
connection being broken and the loss being detected.

* Reorder sip_outbound_registration_state_alloc() so the STATSD_GUAGE's
are still correct if an allocation failure happens.

ASTERISK-27147

Change-Id: I3668405b1ee75dfefb07c0d637826176f741ce83
---
M res/res_pjsip_outbound_registration.c
1 file changed, 105 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/02/6202/1

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 7b605b9..bbb286a 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -358,6 +358,8 @@
 	unsigned int auth_attempted:1;
 	/*! \brief The name of the transport to be used for the registration */
 	char *transport_name;
+	/*! \brief The name of the registration sorcery object */
+	char *registration_name;
 };
 
 /*! \brief Outbound registration state information (persists for lifetime that registration should exist) */
@@ -795,6 +797,82 @@
 	}
 }
 
+static int reregister_immediately_cb(void *obj)
+{
+	struct sip_outbound_registration_state *state = obj;
+
+	if (state->client_state->status != SIP_REGISTRATION_REGISTERED) {
+		ao2_ref(state, -1);
+		return 0;
+	}
+
+	if (DEBUG_ATLEAST(1)) {
+		pjsip_regc_info info;
+
+		pjsip_regc_get_info(state->client_state->client, &info);
+		ast_log(LOG_DEBUG,
+			"Outbound registration transport to server '%.*s' from client '%.*s' shutdown\n",
+			(int) info.server_uri.slen, info.server_uri.ptr,
+			(int) info.client_uri.slen, info.client_uri.ptr);
+	}
+
+	cancel_registration(state->client_state);
+
+	ao2_ref(state->client_state, +1);
+	handle_client_registration(state->client_state);
+
+	ao2_ref(state, -1);
+	return 0;
+}
+
+/*!
+ * \internal
+ * \brief The reliable transport we registered using has shutdown.
+ * \since 13.18.0
+ *
+ * \param obj What is needed to initiate a reregister attempt.
+ *
+ * \return Nothing
+ */
+static void registration_transport_shutdown_cb(void *obj)
+{
+	const char *registration_name = obj;
+	struct sip_outbound_registration_state *state;
+
+	state = get_state(registration_name);
+	if (!state) {
+		/* Registration no longer exists or shutting down. */
+		return;
+	}
+	if (ast_sip_push_task(state->client_state->serializer, reregister_immediately_cb, state)) {
+		ao2_ref(state, -1);
+	}
+}
+
+static void registration_transport_monitor_setup(pjsip_transport *transport, const char *registration_name)
+{
+	char *monitor;
+
+	if (!PJSIP_TRANSPORT_IS_RELIABLE(transport)) {
+		return;
+	}
+	monitor = ao2_alloc_options(strlen(registration_name) + 1, NULL,
+		AO2_ALLOC_OPT_LOCK_NOLOCK);
+	if (!monitor) {
+		return;
+	}
+	strcpy(monitor, registration_name);/* Safe */
+
+	/*
+	 * We'll ignore if the transport has already been shutdown before we
+	 * register the monitor.  We might get into a message spamming infinite
+	 * loop of registration, shutdown, reregistration...
+	 */
+	ast_sip_transport_monitor_register(transport, registration_transport_shutdown_cb,
+		monitor);
+	ao2_ref(monitor, -1);
+}
+
 /*! \brief Callback function for handling a response to a registration attempt */
 static int handle_registration_response(void *data)
 {
@@ -863,9 +941,15 @@
 				next_registration_round = 0;
 			}
 			schedule_registration(response->client_state, next_registration_round);
+
+			/* See if we should monitor for transport shutdown */
+			registration_transport_monitor_setup(response->rdata->tp_info.transport,
+				response->client_state->registration_name);
 		} else {
 			ast_debug(1, "Outbound unregistration to '%s' with client '%s' successful\n", server_uri, client_uri);
 			update_client_state_status(response->client_state, SIP_REGISTRATION_UNREGISTERED);
+			ast_sip_transport_monitor_unregister(response->rdata->tp_info.transport,
+				registration_transport_shutdown_cb);
 		}
 	} else if (response->client_state->destroy) {
 		/* We need to deal with the pending destruction instead. */
@@ -1008,12 +1092,13 @@
 {
 	struct sip_outbound_registration_client_state *client_state = obj;
 
-	ast_free(client_state->transport_name);
 	ast_statsd_log_string("PJSIP.registrations.count", AST_STATSD_GAUGE, "-1", 1.0);
 	ast_statsd_log_string_va("PJSIP.registrations.state.%s", AST_STATSD_GAUGE, "-1", 1.0,
 		sip_outbound_registration_status_str(client_state->status));
 
 	ast_taskprocessor_unreference(client_state->serializer);
+	ast_free(client_state->transport_name);
+	ast_free(client_state->registration_name);
 }
 
 /*! \brief Allocator function for registration state */
@@ -1033,6 +1118,23 @@
 		return NULL;
 	}
 
+	state->client_state->status = SIP_REGISTRATION_UNREGISTERED;
+	state->client_state->timer.user_data = state->client_state;
+	state->client_state->timer.cb = sip_outbound_registration_timer_cb;
+	state->client_state->transport_name = ast_strdup(registration->transport);
+	state->client_state->registration_name =
+		ast_strdup(ast_sorcery_object_get_id(registration));
+
+	ast_statsd_log_string("PJSIP.registrations.count", AST_STATSD_GAUGE, "+1", 1.0);
+	ast_statsd_log_string_va("PJSIP.registrations.state.%s", AST_STATSD_GAUGE, "+1", 1.0,
+		sip_outbound_registration_status_str(state->client_state->status));
+
+	if (!state->client_state->transport_name
+		|| !state->client_state->registration_name) {
+		ao2_cleanup(state);
+		return NULL;
+	}
+
 	/* Create name with seq number appended. */
 	ast_taskprocessor_build_name(tps_name, sizeof(tps_name), "pjsip/outreg/%s",
 		ast_sorcery_object_get_id(registration));
@@ -1043,14 +1145,6 @@
 		ao2_cleanup(state);
 		return NULL;
 	}
-	state->client_state->status = SIP_REGISTRATION_UNREGISTERED;
-	state->client_state->timer.user_data = state->client_state;
-	state->client_state->timer.cb = sip_outbound_registration_timer_cb;
-	state->client_state->transport_name = ast_strdup(registration->transport);
-
-	ast_statsd_log_string("PJSIP.registrations.count", AST_STATSD_GAUGE, "+1", 1.0);
-	ast_statsd_log_string_va("PJSIP.registrations.state.%s", AST_STATSD_GAUGE, "+1", 1.0,
-		sip_outbound_registration_status_str(state->client_state->status));
 
 	state->registration = ao2_bump(registration);
 	return state;
@@ -2054,6 +2148,8 @@
 
 	ao2_global_obj_release(current_states);
 
+	ast_sip_transport_monitor_unregister_all(registration_transport_shutdown_cb);
+
 	/* Wait for registration serializers to get destroyed. */
 	ast_debug(2, "Waiting for registration transactions to complete for unload.\n");
 	remaining = ast_serializer_shutdown_group_join(shutdown_group, MAX_UNLOAD_TIMEOUT_TIME);

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3668405b1ee75dfefb07c0d637826176f741ce83
Gerrit-Change-Number: 6202
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170809/9ac6346f/attachment.html>


More information about the asterisk-code-review mailing list