res_pjsip_outbound_publish: state potential dropped on reloads/realtime fetches

When reloading, or fetching realtime data, if the "apply" failed for any
numerous reasons the current state object would not be maintained. This
potentially resulted in publishes being stopped for some states/clients when
they should not have been.

This patch makes it so the current state object is kept upon any type of reload/
fetch failures.

Change-Id: Iab6020c116d628ed2ae81183e987e2eaa3c90b30

1 files changed, 117 insertions, 36 deletions

diff --git a/res/res_pjsip_outbound_publish.c b/res/res_pjsip_outbound_publish.c
index 60f9bbb17..dd093a518 100644
--- a/res/res_pjsip_outbound_publish.c
+++ b/res/res_pjsip_outbound_publish.c
@@ -406,22 +406,30 @@ static void sip_outbound_publish_synchronize(struct ast_sip_event_publisher_hand
 	ao2_ref(states, -1);
 }
 
-struct ast_sip_outbound_publish_client *ast_sip_publish_client_get(const char *name)
+static struct ast_sip_outbound_publish_state *sip_publish_state_get(const char *id)
 {
-	RAII_VAR(struct ao2_container *, states,
-		 ao2_global_obj_ref(current_states), ao2_cleanup);
-	RAII_VAR(struct ast_sip_outbound_publish_state *, state, NULL, ao2_cleanup);
+	struct ao2_container *states = ao2_global_obj_ref(current_states);
+	struct ast_sip_outbound_publish_state *res;
 
 	if (!states) {
 		return NULL;
 	}
 
-	state = ao2_find(states, name, OBJ_SEARCH_KEY);
+	res = ao2_find(states, id, OBJ_SEARCH_KEY);
+	ao2_ref(states, -1);
+	return res;
+}
+
+struct ast_sip_outbound_publish_client *ast_sip_publish_client_get(const char *name)
+{
+	struct ast_sip_outbound_publish_state *state = sip_publish_state_get(name);
+
 	if (!state) {
 		return NULL;
 	}
 
 	ao2_ref(state->client, +1);
+	ao2_ref(state, -1);
 	return state->client;
 }
 
@@ -1065,25 +1073,89 @@ static struct ast_sip_outbound_publish_state *sip_outbound_publish_state_alloc(
 	return state;
 }
 
-/*! \brief Apply function which finds or allocates a state structure */
-static int sip_outbound_publish_apply(const struct ast_sorcery *sorcery, void *obj)
+static int initialize_publish_client(struct ast_sip_outbound_publish *publish,
+				     struct ast_sip_outbound_publish_state *state)
 {
-	RAII_VAR(struct ao2_container *, states, ao2_global_obj_ref(current_states), ao2_cleanup);
-	RAII_VAR(struct ast_sip_outbound_publish_state *, state, NULL, ao2_cleanup);
-	struct ast_sip_outbound_publish *applied = obj;
+	if (ast_sip_push_task_synchronous(NULL, sip_outbound_publish_client_alloc, state->client)) {
+		ast_log(LOG_ERROR, "Unable to create client for outbound publish '%s'\n",
+			ast_sorcery_object_get_id(publish));
+		return -1;
+	}
 
-	if (ast_strlen_zero(applied->server_uri)) {
+	return 0;
+}
+
+static int validate_publish_config(struct ast_sip_outbound_publish *publish)
+{
+	if (ast_strlen_zero(publish->server_uri)) {
 		ast_log(LOG_ERROR, "No server URI specified on outbound publish '%s'\n",
-			ast_sorcery_object_get_id(applied));
+			ast_sorcery_object_get_id(publish));
 		return -1;
-	} else if (ast_strlen_zero(applied->event)) {
+	} else if (ast_strlen_zero(publish->event)) {
 		ast_log(LOG_ERROR, "No event type specified for outbound publish '%s'\n",
-			ast_sorcery_object_get_id(applied));
+			ast_sorcery_object_get_id(publish));
+		return -1;
+	}
+	return 0;
+}
+
+static int current_state_reusable(struct ast_sip_outbound_publish *publish,
+				  struct ast_sip_outbound_publish_state *current_state)
+{
+	struct ast_sip_outbound_publish *old_publish;
+
+	if (!can_reuse_publish(current_state->client->publish, publish)) {
+		/*
+		 * Something significant has changed in the configuration, so we are
+		 * unable to use the old state object. The current state needs to go
+		 * away and a new one needs to be created.
+		 */
+		return 0;
+	}
+
+	/*
+	 * We can reuse the current state object so keep it, but swap out the
+	 * underlying publish object with the new one.
+	 */
+	old_publish = current_state->client->publish;
+	current_state->client->publish = publish;
+	if (initialize_publish_client(publish, current_state)) {
+		/*
+		 * If the state object fails to re-initialize then swap
+		 * the old publish info back in.
+		 */
+		current_state->client->publish = publish;
 		return -1;
 	}
 
+	/*
+	 * Since we swapped out the publish object the new one needs a ref
+	 * while the old one needs to go away.
+	 */
+	ao2_ref(current_state->client->publish, +1);
+	ao2_cleanup(old_publish);
+
+	/* Tell the caller that the current state object should be used */
+	return 1;
+}
+
+/*! \brief Apply function which finds or allocates a state structure */
+static int sip_outbound_publish_apply(const struct ast_sorcery *sorcery, void *obj)
+{
+#define ADD_TO_NEW_STATES(__obj) \
+	do { if (__obj) { \
+	     ao2_link(new_states, __obj); \
+	     ao2_ref(__obj, -1); } } while (0)
+
+	struct ast_sip_outbound_publish *applied = obj;
+	struct ast_sip_outbound_publish_state *current_state, *new_state;
+	int res;
+
+	/*
+	 * New states are being loaded or reloaded. We'll need to add the new
+	 * object if created/updated, or keep the old object if an error occurs.
+	 */
 	if (!new_states) {
-		/* make sure new_states has been allocated as we will be adding to it */
 		new_states = ao2_container_alloc_options(
 			AO2_ALLOC_OPT_LOCK_NOLOCK, DEFAULT_STATE_BUCKETS,
 			outbound_publish_state_hash, outbound_publish_state_cmp);
@@ -1094,35 +1166,44 @@ static int sip_outbound_publish_apply(const struct ast_sorcery *sorcery, void *o
 		}
 	}
 
-	if (states) {
-		state = ao2_find(states, ast_sorcery_object_get_id(obj), OBJ_SEARCH_KEY);
-		if (state) {
-			if (can_reuse_publish(state->client->publish, applied)) {
-				ao2_replace(state->client->publish, applied);
-			} else {
-				ao2_ref(state, -1);
-				state = NULL;
-			}
-		}
+	/* If there is current state we'll want to maintain it if any errors occur */
+	current_state = sip_publish_state_get(ast_sorcery_object_get_id(applied));
+
+	if ((res = validate_publish_config(applied))) {
+		ADD_TO_NEW_STATES(current_state);
+		return res;
 	}
 
-	if (!state) {
-		state = sip_outbound_publish_state_alloc(applied);
-		if (!state) {
-			ast_log(LOG_ERROR, "Unable to create state for outbound publish '%s'\n",
-				ast_sorcery_object_get_id(applied));
-			return -1;
-		};
+	if (current_state && (res = current_state_reusable(applied, current_state))) {
+		/*
+		 * The current state object was able to be reused, or an error
+		 * occurred. Either way we keep the current state and be done.
+		 */
+		ADD_TO_NEW_STATES(current_state);
+		return res == 1 ? 0 : -1;
 	}
 
-	if (ast_sip_push_task_synchronous(NULL, sip_outbound_publish_client_alloc, state->client)) {
-		ast_log(LOG_ERROR, "Unable to create client for outbound publish '%s'\n",
+	/*
+	 * No current state was found or it was unable to be reused. Either way
+	 * we'll need to create a new state object.
+	 */
+	new_state = sip_outbound_publish_state_alloc(applied);
+	if (!new_state) {
+		ast_log(LOG_ERROR, "Unable to create state for outbound publish '%s'\n",
 			ast_sorcery_object_get_id(applied));
+		ADD_TO_NEW_STATES(current_state);
+		return -1;
+	};
+
+	if (initialize_publish_client(applied, new_state)) {
+		ADD_TO_NEW_STATES(current_state);
+		ao2_ref(new_state, -1);
 		return -1;
 	}
 
-	ao2_link(new_states, state);
-	return 0;
+	ADD_TO_NEW_STATES(new_state);
+	ao2_cleanup(current_state);
+	return res;
 }
 
 static int outbound_auth_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)