[Asterisk-code-review] res pjsip outbound publish: state potential dropped on reloa... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Wed May 11 12:57:34 CDT 2016
Joshua Colp has submitted this change and it was merged.
Change subject: res_pjsip_outbound_publish: state potential dropped on reloads/realtime fetches
......................................................................
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
---
M res/res_pjsip_outbound_publish.c
1 file changed, 121 insertions(+), 40 deletions(-)
Approvals:
George Joseph: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Verified
diff --git a/res/res_pjsip_outbound_publish.c b/res/res_pjsip_outbound_publish.c
index c08737b..51e8a06 100644
--- a/res/res_pjsip_outbound_publish.c
+++ b/res/res_pjsip_outbound_publish.c
@@ -406,22 +406,30 @@
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;
}
@@ -1086,25 +1094,89 @@
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_strlen_zero(applied->server_uri)) {
- ast_log(LOG_ERROR, "No server URI specified on outbound publish '%s'\n",
- ast_sorcery_object_get_id(applied));
- return -1;
- } else if (ast_strlen_zero(applied->event)) {
- ast_log(LOG_ERROR, "No event type specified for outbound publish '%s'\n",
- ast_sorcery_object_get_id(applied));
+ 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;
}
+ 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(publish));
+ return -1;
+ } 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(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);
@@ -1115,35 +1187,44 @@
}
}
- 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)
--
To view, visit https://gerrit.asterisk.org/2754
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab6020c116d628ed2ae81183e987e2eaa3c90b30
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list