<p>neels has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/15833">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fsm: add osmo_fsm_set_term_stops_actions()<br><br>Add optional feature to refuse state changes and event dispatch for FSM<br>instances that are already terminating.<br><br>Enabling this behavior requires osmo_fsm_set_term_stops_actions(true). The<br>default is 'false', which behaves as before this patch, where all events are<br>still dispatched and osmo_fsm_inst_state_chg()es are carried out.<br><br>Rationale:<br><br>Where multiple FSM instances are collaborating (like in osmo-bsc or osmo-msc),<br>a terminating FSM instance often causes events to be dispatched back to itself,<br>or causes state changes in FSM instances that are already terminating. That is<br>hard to avoid, since each FSM instance could be a cause of failure, and wants<br>to notify all the others of that, which in turn often choose to terminate.<br><br>Another use case: any function that dispatches events or state changes to more<br>than one FSM instance must be sure that after the first event dispatch, the<br>second FSM instance is in fact still allocated. Furthermore, if the second FSM<br>instance *has* terminated from the first dispatch, this often means that no<br>more actions should be taken. That could be done by an explicit check for<br>fsm->proc.terminating, but a more general solution is to do this check<br>internally in fsm.c.<br><br>In practice, I need this to avoid a crash in libosmo-mgcp-client, when an<br>on_success() event dispatch causes the MGCP endpoint FSM to deallocate. The<br>earlier dealloc-in-main-loop patch fixed part of it, but not all.<br><br>Change-Id: I0adc13a1a998e953b6c850efa2761350dd07e03a<br>---<br>M include/osmocom/core/fsm.h<br>M src/fsm.c<br>2 files changed, 39 insertions(+), 0 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/15833/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h</span><br><span>index 269befa..8a1999f 100644</span><br><span>--- a/include/osmocom/core/fsm.h</span><br><span>+++ b/include/osmocom/core/fsm.h</span><br><span>@@ -123,6 +123,7 @@</span><br><span> void osmo_fsm_log_timeouts(bool log_timeouts);</span><br><span> void osmo_fsm_term_safely(bool term_safely);</span><br><span> void osmo_fsm_set_dealloc_ctx(void *ctx);</span><br><span style="color: hsl(120, 100%, 40%);">+void osmo_fsm_set_term_stops_actions(bool term_stops_actions);</span><br><span> </span><br><span> /*! Log using FSM instance's context, on explicit logging subsystem and level.</span><br><span> * \param fi An osmo_fsm_inst.</span><br><span>diff --git a/src/fsm.c b/src/fsm.c</span><br><span>index 6aad37a..9c83647 100644</span><br><span>--- a/src/fsm.c</span><br><span>+++ b/src/fsm.c</span><br><span>@@ -104,6 +104,8 @@</span><br><span> void *collect_ctx;</span><br><span> /*! See osmo_fsm_set_dealloc_ctx() */</span><br><span> void *fsm_dealloc_ctx;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! If true, refuse events and state changes on terminating FSM instances. */</span><br><span style="color: hsl(120, 100%, 40%);">+ bool term_stops_actions;</span><br><span> } fsm_term_safely;</span><br><span> </span><br><span> /*! Internal call to free an FSM instance, which redirects to the context set by osmo_fsm_set_dealloc_ctx() if any.</span><br><span>@@ -200,6 +202,27 @@</span><br><span> fsm_term_safely.fsm_dealloc_ctx = ctx;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! For FSM instances that are terminating or already terminated, refuse to dispatch events or make state changes.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Where multiple FSM instances are collaborating, a terminating FSM instance often causes events to be dispatched back</span><br><span style="color: hsl(120, 100%, 40%);">+ * to itself, or causes state changes in FSM instances that are already terminating. That is hard to avoid when each FSM</span><br><span style="color: hsl(120, 100%, 40%);">+ * instance could be a cause of failure, and wants to notify others, which in turn may choose to terminate.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * Another use case: any function that dispatches events or state changes to more than one FSM instance must be sure</span><br><span style="color: hsl(120, 100%, 40%);">+ * that after the first event dispatch, the second FSM instance is in fact still allocated. Furthermore, if the second</span><br><span style="color: hsl(120, 100%, 40%);">+ * FSM instance *has* terminated from the first dispatch, this often means that no more actions should be taken. That</span><br><span style="color: hsl(120, 100%, 40%);">+ * could be done by an explicit check for fsm->proc.terminating, but a more general solution is to do enable this check</span><br><span style="color: hsl(120, 100%, 40%);">+ * internally in fsm.c, with this function.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * The default behavior to still dispatch events and state changes is kept for legacy reasons.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] term_stops_actions If true, refuse actions on terminated FSM instances.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void osmo_fsm_set_term_stops_actions(bool term_stops_actions)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ fsm_term_safely.term_stops_actions = term_stops_actions;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! talloc_free() the given object immediately, or once ongoing FSM terminations are done.</span><br><span> *</span><br><span> * If an FSM deallocation cascade is ongoing, talloc_steal() the given talloc_object into the talloc context that is</span><br><span>@@ -630,6 +653,13 @@</span><br><span> const struct osmo_fsm_state *st = &fsm->states[fi->state];</span><br><span> struct timeval remaining;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (fsm_term_safely.term_stops_actions && fi->proc.terminating) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSMSRC(fi, file, line,</span><br><span style="color: hsl(120, 100%, 40%);">+ "FSM instance already terminating, not changing state to %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_state_name(fsm, new_state));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* validate if new_state is a valid state */</span><br><span> if (!(st->out_state_mask & (1 << new_state))) {</span><br><span> LOGPFSMLSRC(fi, LOGL_ERROR, file, line,</span><br><span>@@ -840,6 +870,14 @@</span><br><span> }</span><br><span> </span><br><span> fsm = fi->fsm;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fsm_term_safely.term_stops_actions && fi->proc.terminating) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSMSRC(fi, file, line,</span><br><span style="color: hsl(120, 100%, 40%);">+ "FSM instance already terminating, not dispatching event %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_event_name(fsm, event));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> OSMO_ASSERT(fi->state < fsm->num_states);</span><br><span> fs = &fi->fsm->states[fi->state];</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/15833">change 15833</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.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.osmocom.org/c/libosmocore/+/15833"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I0adc13a1a998e953b6c850efa2761350dd07e03a </div>
<div style="display:none"> Gerrit-Change-Number: 15833 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>