Merge "res_pjsip_registrar: blocked threads on reliable transport shutdown take 3"

include/asterisk/res_pjsip.h

@@ -3245,6 +3245,29 @@ enum ast_transport_monitor_reg {
 enum ast_transport_monitor_reg ast_sip_transport_monitor_register(pjsip_transport *transport,
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data);
 
+/*!
+ * \brief Register a reliable transport shutdown monitor callback replacing any duplicate.
+ * \since 13.26.0
+ * \since 16.3.0
+ *
+ * \param transport Transport to monitor for shutdown.
+ * \param cb Who to call when transport is shutdown.
+ * \param ao2_data Data to pass with the callback.
+ * \param matches Matcher function that returns true if data matches a previously
+ *                registered data object
+ *
+ * \note The data object passed will have its reference count automatically
+ * incremented by this call and automatically decremented after the callback
+ * runs or when the callback is unregistered.
+ *
+ * This function checks for duplicates, and overwrites/replaces the old monitor
+ * with the given one.
+ *
+ * \return enum ast_transport_monitor_reg
+ */
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_replace(pjsip_transport *transport,
+	ast_transport_monitor_shutdown_cb cb, void *ao2_data, ast_transport_monitor_data_matcher matches);
+
 /*!
  * \brief Unregister a reliable transport shutdown monitor
  * \since 13.20.0

res/res_pjsip/pjsip_transport_events.c

@@ -305,6 +305,12 @@ void ast_sip_transport_monitor_unregister(pjsip_transport *transport,
 
 enum ast_transport_monitor_reg ast_sip_transport_monitor_register(pjsip_transport *transport,
 	ast_transport_monitor_shutdown_cb cb, void *ao2_data)
+{
+	return ast_sip_transport_monitor_register_replace(transport, cb, ao2_data, NULL);
+}
+
+enum ast_transport_monitor_reg ast_sip_transport_monitor_register_replace(pjsip_transport *transport,
+	ast_transport_monitor_shutdown_cb cb, void *ao2_data, ast_transport_monitor_data_matcher matches)
 {
 	struct ao2_container *transports;
 	struct transport_monitor *monitored;
@@ -321,6 +327,13 @@ enum ast_transport_monitor_reg ast_sip_transport_monitor_register(pjsip_transpor
 	monitored = ao2_find(transports, transport->obj_name, OBJ_SEARCH_KEY | OBJ_NOLOCK);
 	if (monitored) {
 		struct transport_monitor_notifier new_monitor;
+		struct callback_data cb_data = {
+			.cb = cb,
+			.data = ao2_data,
+			.matches = matches ?: ptr_matcher,
+		};
+
+		transport_monitor_unregister_cb(monitored, &cb_data, 0);
 
 		/* Add new monitor to vector */
 		new_monitor.cb = cb;

res/res_pjsip_registrar.c

@@ -197,30 +197,22 @@ static int registrar_validate_contacts(const pjsip_rx_data *rdata, pj_pool_t *po
 	return 0;
 }
 
+enum contact_delete_type {
+	CONTACT_DELETE_ERROR,
+	CONTACT_DELETE_EXISTING,
+	CONTACT_DELETE_EXPIRE,
+	CONTACT_DELETE_REQUEST,
+	CONTACT_DELETE_SHUTDOWN,
+};
+
+static int registrar_contact_delete(enum contact_delete_type type, pjsip_transport *transport,
+	struct ast_sip_contact *contact, const char *aor_name);
+
 /*! \brief Internal function used to delete a contact from an AOR */
 static int registrar_delete_contact(void *obj, void *arg, int flags)
 {
-	struct ast_sip_contact *contact = obj;
+	return registrar_contact_delete(
-	const char *aor_name = arg;
+		CONTACT_DELETE_REQUEST, NULL, obj, arg) ? 0 : CMP_MATCH;
-
-	/* Permanent contacts can't be deleted */
-	if (ast_tvzero(contact->expiration_time)) {
-		return 0;
-	}
-
-	ast_sip_location_delete_contact(contact);
-	if (!ast_strlen_zero(aor_name)) {
-		ast_verb(3, "Removed contact '%s' from AOR '%s' due to request\n", contact->uri, aor_name);
-		ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
-				"Contact: %s\r\n"
-				"AOR: %s\r\n"
-				"UserAgent: %s",
-				contact->uri,
-				aor_name,
-				contact->user_agent);
-	}
-
-	return CMP_MATCH;
 }
 
 /*! \brief Internal function which adds a contact to a response */
@@ -352,16 +344,7 @@ static int register_contact_transport_remove_cb(void *data)
 
 	contact = ast_sip_location_retrieve_contact(monitor->contact_name);
 	if (contact) {
-		ast_sip_location_delete_contact(contact);
+		registrar_contact_delete(CONTACT_DELETE_SHUTDOWN, NULL, contact, monitor->aor_name);
-		ast_verb(3, "Removed contact '%s' from AOR '%s' due to transport shutdown\n",
-			contact->uri, monitor->aor_name);
-		ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
-			"Contact: %s\r\n"
-			"AOR: %s\r\n"
-			"UserAgent: %s",
-			contact->uri,
-			monitor->aor_name,
-			contact->user_agent);
 		ao2_ref(contact, -1);
 	}
 	ao2_unlock(aor);
@@ -414,6 +397,81 @@ static void register_contact_transport_shutdown_cb(void *data)
 	ao2_unlock(monitor);
 }
 
+
+static int registrar_contact_delete(enum contact_delete_type type, pjsip_transport *transport,
+	struct ast_sip_contact *contact, const char *aor_name)
+{
+	int aor_size;
+
+	/* Permanent contacts can't be deleted */
+	if (ast_tvzero(contact->expiration_time)) {
+		return -1;
+	}
+
+	aor_size = aor_name ? strlen(aor_name) : 0;
+	if (contact->prune_on_boot && type != CONTACT_DELETE_SHUTDOWN && aor_size) {
+		const char *contact_name = ast_sorcery_object_get_id(contact);
+		struct contact_transport_monitor *monitor = ast_alloca(
+			sizeof(*monitor) + 2 + aor_size + strlen(contact_name));
+
+		strcpy(monitor->aor_name, aor_name); /* Safe */
+		monitor->contact_name = monitor->aor_name + aor_size + 1;
+		strcpy(monitor->contact_name, contact_name); /* Safe */
+
+		if (transport) {
+			ast_sip_transport_monitor_unregister(transport,
+				register_contact_transport_shutdown_cb,	monitor,
+				contact_transport_monitor_matcher);
+		} else {
+			/*
+			 * If a specific transport is not supplied then unregister the matching
+			 * monitor from all reliable transports.
+			 */
+			ast_sip_transport_monitor_unregister_all(register_contact_transport_shutdown_cb,
+				monitor, contact_transport_monitor_matcher);
+		}
+	}
+
+	ast_sip_location_delete_contact(contact);
+
+	if (aor_size) {
+		if (VERBOSITY_ATLEAST(3)) {
+			const char *reason = "none";
+
+			switch (type) {
+			case CONTACT_DELETE_ERROR:
+				reason = "registration failure";
+				break;
+			case CONTACT_DELETE_EXISTING:
+				reason = "remove existing";
+				break;
+			case CONTACT_DELETE_EXPIRE:
+				reason = "expiration";
+				break;
+			case CONTACT_DELETE_REQUEST:
+				reason = "request";
+				break;
+			case CONTACT_DELETE_SHUTDOWN:
+				reason = "shutdown";
+				break;
+			}
+
+			ast_verb(3, "Removed contact '%s' from AOR '%s' due to %s\n",
+					 contact->uri, aor_name, reason);
+		}
+
+		ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
+				"Contact: %s\r\n"
+				"AOR: %s\r\n"
+				"UserAgent: %s",
+				contact->uri,
+				aor_name,
+				contact->user_agent);
+	}
+
+	return 0;
+}
+
 AST_VECTOR(excess_contact_vector, struct ast_sip_contact *);
 
 static int vec_contact_cmp(struct ast_sip_contact *left, struct ast_sip_contact *right)
@@ -490,16 +548,7 @@ static void remove_excess_contacts(struct ao2_container *contacts, struct ao2_co
 
 		contact = AST_VECTOR_GET(&contact_vec, to_remove);
 
-		ast_sip_location_delete_contact(contact);
+		registrar_contact_delete(CONTACT_DELETE_EXISTING, NULL, contact, contact->aor);
-		ast_verb(3, "Removed contact '%s' from AOR '%s' due to remove_existing\n",
-			contact->uri, contact->aor);
-		ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
-			"Contact: %s\r\n"
-			"AOR: %s\r\n"
-			"UserAgent: %s",
-			contact->uri,
-			contact->aor,
-			contact->user_agent);
 
 		ao2_unlink(response_contacts, contact);
 	}
@@ -729,8 +778,8 @@ static void register_aor_core(pjsip_rx_data *rdata,
 					monitor->contact_name = monitor->aor_name + strlen(aor_name) + 1;
 					strcpy(monitor->contact_name, contact_name);/* Safe */
 
-					ast_sip_transport_monitor_register(rdata->tp_info.transport,
+					ast_sip_transport_monitor_register_replace(rdata->tp_info.transport,
-						register_contact_transport_shutdown_cb, monitor);
+						register_contact_transport_shutdown_cb, monitor, contact_transport_monitor_matcher);
 					ao2_ref(monitor, -1);
 				}
 			}
@@ -774,7 +823,8 @@ static void register_aor_core(pjsip_rx_data *rdata,
 			if (ast_sip_location_update_contact(contact_update)) {
 				ast_log(LOG_ERROR, "Failed to update contact '%s' expiration time to %d seconds.\n",
 					contact->uri, expiration);
-				ast_sip_location_delete_contact(contact);
+				registrar_contact_delete(CONTACT_DELETE_ERROR, rdata->tp_info.transport,
+					contact, aor_name);
 				continue;
 			}
 			ast_debug(3, "Refreshed contact '%s' on AOR '%s' with new expiration of %d seconds\n",
@@ -791,31 +841,8 @@ static void register_aor_core(pjsip_rx_data *rdata,
 			ao2_link(contacts, contact_update);
 			ao2_cleanup(contact_update);
 		} else {
-			if (contact->prune_on_boot) {
+			registrar_contact_delete(CONTACT_DELETE_REQUEST, rdata->tp_info.transport,
-				struct contact_transport_monitor *monitor;
+					contact, aor_name);
-				const char *contact_name =
-					ast_sorcery_object_get_id(contact);
-
-				monitor = ast_alloca(sizeof(*monitor) + 2 + strlen(aor_name)
-					+ strlen(contact_name));
-				strcpy(monitor->aor_name, aor_name);/* Safe */
-				monitor->contact_name = monitor->aor_name + strlen(aor_name) + 1;
-				strcpy(monitor->contact_name, contact_name);/* Safe */
-
-				ast_sip_transport_monitor_unregister(rdata->tp_info.transport,
-					register_contact_transport_shutdown_cb, monitor, contact_transport_monitor_matcher);
-			}
-
-			/* We want to report the user agent that was actually in the removed contact */
-			ast_sip_location_delete_contact(contact);
-			ast_verb(3, "Removed contact '%s' from AOR '%s' due to request\n", contact_uri, aor_name);
-			ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
-					"Contact: %s\r\n"
-					"AOR: %s\r\n"
-					"UserAgent: %s",
-					contact_uri,
-					aor_name,
-					contact->user_agent);
 		}
 	}
 
@@ -1212,20 +1239,7 @@ static int expire_contact(void *obj, void *arg, int flags)
 	 */
 	ao2_lock(lock);
 	if (ast_tvdiff_ms(ast_tvnow(), contact->expiration_time) > 0) {
-		if (contact->prune_on_boot) {
+		registrar_contact_delete(CONTACT_DELETE_EXPIRE, NULL, contact, contact->aor);
-			struct contact_transport_monitor *monitor;
-			const char *contact_name = ast_sorcery_object_get_id(contact);
-
-			monitor = ast_alloca(sizeof(*monitor) + 2 + strlen(contact->aor)
-				+ strlen(contact_name));
-			strcpy(monitor->aor_name, contact->aor);/* Safe */
-			monitor->contact_name = monitor->aor_name + strlen(contact->aor) + 1;
-			strcpy(monitor->contact_name, contact_name);/* Safe */
-
-			ast_sip_transport_monitor_unregister_all(register_contact_transport_shutdown_cb,
-				monitor, contact_transport_monitor_matcher);
-		}
-		ast_sip_location_delete_contact(contact);
 	}
 	ao2_unlock(lock);
 	ast_named_lock_put(lock);