<p>George Joseph <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/12911">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, approved; Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">stasis_state: Create internal stasis_state_proxy object.<br><br>This improves the way which stasis_state reference counting works.<br>Since manager->states holds onto the proxy object instead of the real<br>object this allows stasis_state objects to be freed when appropriate<br>without use of a special state_remove function. Additionally each<br>distinct eid associated with the state holds a reference to the state to<br>prevent early release and potentially allow easier debug of leaks.<br><br>Change-Id: I400e0db4b9afa3d5cb4ac7dad60907897e73f9a9<br>---<br>M main/stasis_state.c<br>1 file changed, 147 insertions(+), 171 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/stasis_state.c b/main/stasis_state.c</span><br><span>index aa00f9a..b85462f 100644</span><br><span>--- a/main/stasis_state.c</span><br><span>+++ b/main/stasis_state.c</span><br><span>@@ -26,6 +26,18 @@</span><br><span> </span><br><span> /*!</span><br><span> * \internal</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Used to link a stasis_state to it's manager</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+struct stasis_state_proxy {</span><br><span style="color: hsl(120, 100%, 40%);">+ AO2_WEAKPROXY();</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! The manager that owns and handles this state */</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state_manager *manager;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! A unique id for this state object. */</span><br><span style="color: hsl(120, 100%, 40%);">+ char id[0];</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \internal</span><br><span> * \brief Associates a stasis topic to its last known published message</span><br><span> *</span><br><span> * This object's lifetime is tracked by the number of publishers and subscribers to it.</span><br><span>@@ -38,7 +50,10 @@</span><br><span> struct stasis_state {</span><br><span> /*! The number of state subscribers */</span><br><span> unsigned int num_subscribers;</span><br><span style="color: hsl(0, 100%, 40%);">- /*! The manager that owns and handles this state */</span><br><span style="color: hsl(120, 100%, 40%);">+ /*!</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief The manager that owns and handles this state</span><br><span style="color: hsl(120, 100%, 40%);">+ * \note This reference is owned by stasis_state_proxy</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span> struct stasis_state_manager *manager;</span><br><span> /*! Forwarding information, i.e. this topic to manager's topic */</span><br><span> struct stasis_forward *forward;</span><br><span>@@ -52,11 +67,11 @@</span><br><span> */</span><br><span> AST_VECTOR(, struct ast_eid) eids;</span><br><span> /*! A unique id for this state object. */</span><br><span style="color: hsl(0, 100%, 40%);">- char id[0];</span><br><span style="color: hsl(120, 100%, 40%);">+ char *id;</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-AO2_STRING_FIELD_HASH_FN(stasis_state, id);</span><br><span style="color: hsl(0, 100%, 40%);">-AO2_STRING_FIELD_CMP_FN(stasis_state, id);</span><br><span style="color: hsl(120, 100%, 40%);">+AO2_STRING_FIELD_HASH_FN(stasis_state_proxy, id);</span><br><span style="color: hsl(120, 100%, 40%);">+AO2_STRING_FIELD_CMP_FN(stasis_state_proxy, id);</span><br><span> </span><br><span> /*! The number of buckets to use for managed states */</span><br><span> #define STATE_BUCKETS 57</span><br><span>@@ -112,17 +127,28 @@</span><br><span> state->topic = NULL;</span><br><span> ao2_cleanup(state->msg);</span><br><span> state->msg = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_cleanup(state->manager);</span><br><span style="color: hsl(0, 100%, 40%);">- state->manager = NULL;</span><br><span> </span><br><span> /* All eids should have been removed */</span><br><span> ast_assert(AST_VECTOR_SIZE(&state->eids) == 0);</span><br><span> AST_VECTOR_FREE(&state->eids);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void state_proxy_dtor(void *obj) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state_proxy *proxy = obj;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(proxy->manager);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static void state_proxy_sub_cb(void *obj, void *data)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state_proxy *proxy = obj;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_unlink(proxy->manager->states, proxy);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*!</span><br><span> * \internal</span><br><span style="color: hsl(0, 100%, 40%);">- * \brief Allocate a stasis state object.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \brief Allocate a stasis state object and add it to the manager.</span><br><span> *</span><br><span> * Create and initialize a state structure. It's required that either a state</span><br><span> * topic, or an id is specified. If a state topic is not given then one will be</span><br><span>@@ -134,45 +160,16 @@</span><br><span> *</span><br><span> * \return A stasis_state object or NULL</span><br><span> * \return NULL on error</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \pre manager->states must be locked.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \pre manager->states does not contain an object matching key \a id.</span><br><span> */</span><br><span> static struct stasis_state *state_alloc(struct stasis_state_manager *manager,</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_topic *state_topic, const char *id)</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_topic *state_topic, const char *id,</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *file, int line, const char *func)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_state *state;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!state_topic) {</span><br><span style="color: hsl(0, 100%, 40%);">- char *name;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* If not given a state topic, then an id is required */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_assert(id != NULL);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * To provide further detail and to ensure that the topic is unique within the</span><br><span style="color: hsl(0, 100%, 40%);">- * scope of the system we prefix it with the manager's topic name, which should</span><br><span style="color: hsl(0, 100%, 40%);">- * itself already be unique.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if (ast_asprintf(&name, "%s/%s", stasis_topic_name(manager->all_topic), id) < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to create state topic name '%s/%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- stasis_topic_name(manager->all_topic), id);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- state_topic = stasis_topic_create(name);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!state_topic) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to create state topic '%s'\n", name);</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(name);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- ast_free(name);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * Since the state topic was passed in, go ahead and bump its reference.</span><br><span style="color: hsl(0, 100%, 40%);">- * By doing this here first, it allows us to consistently decrease the reference on</span><br><span style="color: hsl(0, 100%, 40%);">- * state allocation error.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state_topic, +1);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state_proxy *proxy = NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state *state = NULL;</span><br><span> </span><br><span> if (!id) {</span><br><span> /* If not given an id, then a state topic is required */</span><br><span>@@ -182,77 +179,87 @@</span><br><span> id = state_id_by_topic(manager->all_topic, state_topic);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state = ao2_alloc(sizeof(*state) + strlen(id) + 1, state_dtor);</span><br><span style="color: hsl(120, 100%, 40%);">+ state = __ao2_alloc(sizeof(*state), state_dtor, AO2_ALLOC_OPT_LOCK_MUTEX, id, file, line, func);</span><br><span> if (!state) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to allocate state '%s' in manager '%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- id, stasis_topic_name(manager->all_topic));</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state_topic, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- strcpy(state->id, id); /* Safe */</span><br><span style="color: hsl(0, 100%, 40%);">- state->topic = state_topic; /* ref already bumped above */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!state_topic) {</span><br><span style="color: hsl(120, 100%, 40%);">+ char *name;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * To provide further detail and to ensure that the topic is unique within the</span><br><span style="color: hsl(120, 100%, 40%);">+ * scope of the system we prefix it with the manager's topic name, which should</span><br><span style="color: hsl(120, 100%, 40%);">+ * itself already be unique.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_asprintf(&name, "%s/%s", stasis_topic_name(manager->all_topic), id) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ state->topic = stasis_topic_create(name);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_free(name);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!state->topic) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ /*</span><br><span style="color: hsl(120, 100%, 40%);">+ * Since the state topic was passed in, go ahead and bump its reference.</span><br><span style="color: hsl(120, 100%, 40%);">+ * By doing this here first, it allows us to consistently decrease the reference on</span><br><span style="color: hsl(120, 100%, 40%);">+ * state allocation error.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(state_topic, +1);</span><br><span style="color: hsl(120, 100%, 40%);">+ state->topic = state_topic;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ proxy = ao2_t_weakproxy_alloc(sizeof(*proxy) + strlen(id) + 1, state_proxy_dtor, id);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!proxy) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ strcpy(proxy->id, id); /* Safe */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ state->id = proxy->id;</span><br><span style="color: hsl(120, 100%, 40%);">+ proxy->manager = ao2_bump(manager);</span><br><span style="color: hsl(120, 100%, 40%);">+ state->manager = proxy->manager; /* state->manager is owned by the proxy */</span><br><span> </span><br><span> state->forward = stasis_forward_all(state->topic, manager->all_topic);</span><br><span> if (!state->forward) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to add state '%s' forward in manager '%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- id, stasis_topic_name(manager->all_topic));</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span> }</span><br><span> </span><br><span> if (AST_VECTOR_INIT(&state->eids, 2)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to initialize eids for state '%s' in manager '%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- id, stasis_topic_name(manager->all_topic));</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state->manager = ao2_bump(manager);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ao2_t_weakproxy_set_object(proxy, state, OBJ_NOLOCK, "weakproxy link")) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ao2_weakproxy_subscribe(proxy, state_proxy_sub_cb, NULL, OBJ_NOLOCK)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!ao2_link_flags(manager->states, proxy, OBJ_NOLOCK)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ goto error_return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(proxy, -1);</span><br><span> </span><br><span> return state;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*!</span><br><span style="color: hsl(0, 100%, 40%);">- * \internal</span><br><span style="color: hsl(0, 100%, 40%);">- * \brief Create a state object, and add it to the manager.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \note Locking on the states container is specifically not done here, thus</span><br><span style="color: hsl(0, 100%, 40%);">- * appropriate locks should be applied prior to this function being called.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \param manager The manager to be added to</span><br><span style="color: hsl(0, 100%, 40%);">- * \param state_topic A state topic to be managed (if NULL id is required)</span><br><span style="color: hsl(0, 100%, 40%);">- * \param id The unique id for the state (if NULL state_topic is required)</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \return The added state object</span><br><span style="color: hsl(0, 100%, 40%);">- * \return NULL on error</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-static struct stasis_state *state_add(struct stasis_state_manager *manager,</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_topic *state_topic, const char *id)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_state *state = state_alloc(manager, state_topic, id);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!state) {</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!ao2_link_flags(manager->states, state, OBJ_NOLOCK)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_ERROR, "Unable to add state '%s' to manager '%s'\n",</span><br><span style="color: hsl(0, 100%, 40%);">- state->id ? state->id : "", stasis_topic_name(manager->all_topic));</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- return NULL;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- return state;</span><br><span style="color: hsl(120, 100%, 40%);">+error_return:</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_ERROR, "Unable to allocate state '%s' in manager '%s'\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ id, stasis_topic_name(manager->all_topic));</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(state);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(proxy);</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span> }</span><br><span> </span><br><span> /*!</span><br><span> * \internal</span><br><span> * \brief Find a state by id, or create one if not found and add it to the manager.</span><br><span> *</span><br><span style="color: hsl(0, 100%, 40%);">- * \note Locking on the states container is specifically not done here, thus</span><br><span style="color: hsl(0, 100%, 40%);">- * appropriate locks should be applied prior to this function being called.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span> * \param manager The manager to be added to</span><br><span> * \param state_topic A state topic to be managed (if NULL id is required)</span><br><span> * \param id The unique id for the state (if NULL state_topic is required)</span><br><span>@@ -260,18 +267,26 @@</span><br><span> * \return The added state object</span><br><span> * \return NULL on error</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">-static struct stasis_state *state_find_or_add(struct stasis_state_manager *manager,</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_topic *state_topic, const char *id)</span><br><span style="color: hsl(120, 100%, 40%);">+#define state_find_or_add(mgr, top, id) __state_find_or_add(mgr, top, id, __FILE__, __LINE__, __PRETTY_FUNCTION__)</span><br><span style="color: hsl(120, 100%, 40%);">+static struct stasis_state *__state_find_or_add(struct stasis_state_manager *manager,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_topic *state_topic, const char *id,</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *file, int line, const char *func)</span><br><span> {</span><br><span> struct stasis_state *state;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_lock(manager->states);</span><br><span> if (ast_strlen_zero(id)) {</span><br><span> id = state_id_by_topic(manager->all_topic, state_topic);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state = ao2_find(manager->states, id, OBJ_SEARCH_KEY | OBJ_NOLOCK);</span><br><span style="color: hsl(120, 100%, 40%);">+ state = ao2_weakproxy_find(manager->states, id, OBJ_SEARCH_KEY | OBJ_NOLOCK, "");</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!state) {</span><br><span style="color: hsl(120, 100%, 40%);">+ state = state_alloc(manager, state_topic, id, file, line, func);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return state ? state : state_add(manager, state_topic, id);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_unlock(manager->states);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return state;</span><br><span> }</span><br><span> </span><br><span> static void state_manager_dtor(void *obj)</span><br><span>@@ -317,7 +332,7 @@</span><br><span> }</span><br><span> </span><br><span> manager->states = ao2_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0,</span><br><span style="color: hsl(0, 100%, 40%);">- STATE_BUCKETS, stasis_state_hash_fn, NULL, stasis_state_cmp_fn);</span><br><span style="color: hsl(120, 100%, 40%);">+ STATE_BUCKETS, stasis_state_proxy_hash_fn, NULL, stasis_state_proxy_cmp_fn);</span><br><span> if (!manager->states) {</span><br><span> ao2_ref(manager, -1);</span><br><span> return NULL;</span><br><span>@@ -356,10 +371,7 @@</span><br><span> struct stasis_topic *topic;</span><br><span> struct stasis_state *state;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(manager->states);</span><br><span> state = state_find_or_add(manager, NULL, id);</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!state) {</span><br><span> return NULL;</span><br><span> }</span><br><span>@@ -369,53 +381,6 @@</span><br><span> return topic;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*!</span><br><span style="color: hsl(0, 100%, 40%);">- * \internal</span><br><span style="color: hsl(0, 100%, 40%);">- * \brief Remove a state from the stasis manager</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * State should only be removed from the manager under the following conditions:</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * There are no more subscribers to it</span><br><span style="color: hsl(0, 100%, 40%);">- * There are no more explicit publishers publishing to it</span><br><span style="color: hsl(0, 100%, 40%);">- * There are no more implicit publishers publishing to it</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * Subscribers and explicit publishers hold a reference to the state object itself, so</span><br><span style="color: hsl(0, 100%, 40%);">- * once a state's reference count drops to 2 (1 for the manager, 1 passed in) then we</span><br><span style="color: hsl(0, 100%, 40%);">- * know there are no more subscribers or explicit publishers. Implicit publishers are</span><br><span style="color: hsl(0, 100%, 40%);">- * tracked by eids, so once that container is empty no more implicit publishers exist</span><br><span style="color: hsl(0, 100%, 40%);">- * for the state either. Only then can a state be removed.</span><br><span style="color: hsl(0, 100%, 40%);">- *</span><br><span style="color: hsl(0, 100%, 40%);">- * \param state The state to remove</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">-static void state_remove(struct stasis_state *state)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(state);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * The manager's state container also needs to be locked here prior to checking</span><br><span style="color: hsl(0, 100%, 40%);">- * the state's reference count, and potentially removing since we don't want its</span><br><span style="color: hsl(0, 100%, 40%);">- * count to possibly increase between the check and unlinking.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(state->manager->states);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /*</span><br><span style="color: hsl(0, 100%, 40%);">- * If there are only 2 references left then it's the one owned by the manager,</span><br><span style="color: hsl(0, 100%, 40%);">- * and the one passed in to this function. However, before removing it from the</span><br><span style="color: hsl(0, 100%, 40%);">- * manager we need to also check that no eid is associated with the given state.</span><br><span style="color: hsl(0, 100%, 40%);">- * If an eid still remains then this means that an implicit publisher is still</span><br><span style="color: hsl(0, 100%, 40%);">- * publishing to this state.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if (ao2_ref(state, 0) == 2 && AST_VECTOR_SIZE(&state->eids) == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlink_flags(state->manager->states, state, 0);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(state->manager->states);</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(state);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* Now it's safe to remove the reference that is held on the given state */</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_ref(state, -1);</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> struct stasis_state_subscriber {</span><br><span> /*! The stasis state subscribed to */</span><br><span> struct stasis_state *state;</span><br><span>@@ -441,7 +406,7 @@</span><br><span> --sub->state->num_subscribers;</span><br><span> ao2_unlock(sub->state);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state_remove(sub->state);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(sub->state, -1);</span><br><span> }</span><br><span> </span><br><span> struct stasis_state_subscriber *stasis_state_add_subscriber(</span><br><span>@@ -457,14 +422,11 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(manager->states);</span><br><span> sub->state = state_find_or_add(manager, NULL, id);</span><br><span> if (!sub->state) {</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span> ao2_ref(sub, -1);</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span> </span><br><span> ao2_lock(sub->state);</span><br><span> ++sub->state->num_subscribers;</span><br><span>@@ -563,7 +525,7 @@</span><br><span> {</span><br><span> struct stasis_state_publisher *pub = obj;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state_remove(pub->state);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(pub->state, -1);</span><br><span> }</span><br><span> </span><br><span> struct stasis_state_publisher *stasis_state_add_publisher(</span><br><span>@@ -578,14 +540,11 @@</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(manager->states);</span><br><span> pub->state = state_find_or_add(manager, NULL, id);</span><br><span> if (!pub->state) {</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span> ao2_ref(pub, -1);</span><br><span> return NULL;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span> </span><br><span> return pub;</span><br><span> }</span><br><span>@@ -639,7 +598,10 @@</span><br><span> }</span><br><span> </span><br><span> if (i == AST_VECTOR_SIZE(&state->eids)) {</span><br><span style="color: hsl(0, 100%, 40%);">- AST_VECTOR_APPEND(&state->eids, *eid);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!AST_VECTOR_APPEND(&state->eids, *eid)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* This ensures state cannot be freed if it has any eids */</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(state, +1);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>@@ -666,6 +628,8 @@</span><br><span> for (i = 0; i < AST_VECTOR_SIZE(&state->eids); ++i) {</span><br><span> if (!ast_eid_cmp(AST_VECTOR_GET_ADDR(&state->eids, i), eid)) {</span><br><span> AST_VECTOR_REMOVE_UNORDERED(&state->eids, i);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Balance the reference from state_find_or_add_eid */</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(state, -1);</span><br><span> return;</span><br><span> }</span><br><span> }</span><br><span>@@ -676,10 +640,7 @@</span><br><span> {</span><br><span> struct stasis_state *state;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ao2_lock(manager->states);</span><br><span> state = state_find_or_add(manager, NULL, id);</span><br><span style="color: hsl(0, 100%, 40%);">- ao2_unlock(manager->states);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!state) {</span><br><span> return;</span><br><span> }</span><br><span>@@ -697,7 +658,7 @@</span><br><span> void stasis_state_remove_publish_by_id(struct stasis_state_manager *manager,</span><br><span> const char *id, const struct ast_eid *eid, struct stasis_message *msg)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_state *state = ao2_find(manager->states, id, OBJ_SEARCH_KEY);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state *state = ao2_weakproxy_find(manager->states, id, OBJ_SEARCH_KEY, "");</span><br><span> </span><br><span> if (!state) {</span><br><span> /*</span><br><span>@@ -721,7 +682,7 @@</span><br><span> state_find_and_remove_eid(state, eid);</span><br><span> ao2_unlock(state);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- state_remove(state);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(state, -1);</span><br><span> }</span><br><span> </span><br><span> int stasis_state_add_observer(struct stasis_state_manager *manager,</span><br><span>@@ -744,10 +705,8 @@</span><br><span> AST_VECTOR_RW_UNLOCK(&manager->observers);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int handle_stasis_state(void *obj, void *arg, void *data, int flags)</span><br><span style="color: hsl(120, 100%, 40%);">+static int handle_stasis_state(struct stasis_state *state, on_stasis_state handler, void *data)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_state *state = obj;</span><br><span style="color: hsl(0, 100%, 40%);">- on_stasis_state handler = arg;</span><br><span> struct stasis_message *msg;</span><br><span> int res;</span><br><span> </span><br><span>@@ -764,24 +723,41 @@</span><br><span> return res;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int handle_stasis_state_proxy(void *obj, void *arg, void *data, int flags)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state *state = ao2_weakproxy_get_object(obj, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (state) {</span><br><span style="color: hsl(120, 100%, 40%);">+ int res;</span><br><span style="color: hsl(120, 100%, 40%);">+ res = handle_stasis_state(state, arg, data);</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_ref(state, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+ return res;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> void stasis_state_callback_all(struct stasis_state_manager *manager, on_stasis_state handler,</span><br><span> void *data)</span><br><span> {</span><br><span> ast_assert(handler != NULL);</span><br><span> </span><br><span> ao2_callback_data(manager->states, OBJ_MULTIPLE | OBJ_NODATA,</span><br><span style="color: hsl(0, 100%, 40%);">- handle_stasis_state, handler, data);</span><br><span style="color: hsl(120, 100%, 40%);">+ handle_stasis_state_proxy, handler, data);</span><br><span> }</span><br><span> </span><br><span> static int handle_stasis_state_subscribed(void *obj, void *arg, void *data, int flags)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct stasis_state *state = obj;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct stasis_state *state = ao2_weakproxy_get_object(obj, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ int res = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (state->num_subscribers) {</span><br><span style="color: hsl(0, 100%, 40%);">- return handle_stasis_state(obj, arg, data, flags);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (state && state->num_subscribers) {</span><br><span style="color: hsl(120, 100%, 40%);">+ res = handle_stasis_state(state, arg, data);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ ao2_cleanup(state);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return res;</span><br><span> }</span><br><span> </span><br><span> void stasis_state_callback_subscribed(struct stasis_state_manager *manager, on_stasis_state handler,</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/12911">change 12911</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/12911"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 17 </div>
<div style="display:none"> Gerrit-Change-Id: I400e0db4b9afa3d5cb4ac7dad60907897e73f9a9 </div>
<div style="display:none"> Gerrit-Change-Number: 12911 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>