Change in libosmocore[master]: fsm: add osmo_fsm_set_term_stops_actions()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

neels gerrit-no-reply at lists.osmocom.org
Wed Oct 23 01:41:51 UTC 2019


neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/15833 )


Change subject: fsm: add osmo_fsm_set_term_stops_actions()
......................................................................

fsm: add osmo_fsm_set_term_stops_actions()

Add optional feature to refuse state changes and event dispatch for FSM
instances that are already terminating.

Enabling this behavior requires osmo_fsm_set_term_stops_actions(true). The
default is 'false', which behaves as before this patch, where all events are
still dispatched and osmo_fsm_inst_state_chg()es are carried out.

Rationale:

Where multiple FSM instances are collaborating (like in osmo-bsc or osmo-msc),
a terminating FSM instance often causes events to be dispatched back to itself,
or causes state changes in FSM instances that are already terminating. That is
hard to avoid, since each FSM instance could be a cause of failure, and wants
to notify all the others of that, which in turn often choose to terminate.

Another use case: any function that dispatches events or state changes to more
than one FSM instance must be sure that after the first event dispatch, the
second FSM instance is in fact still allocated. Furthermore, if the second FSM
instance *has* terminated from the first dispatch, this often means that no
more actions should be taken. That could be done by an explicit check for
fsm->proc.terminating, but a more general solution is to do this check
internally in fsm.c.

In practice, I need this to avoid a crash in libosmo-mgcp-client, when an
on_success() event dispatch causes the MGCP endpoint FSM to deallocate. The
earlier dealloc-in-main-loop patch fixed part of it, but not all.

Change-Id: I0adc13a1a998e953b6c850efa2761350dd07e03a
---
M include/osmocom/core/fsm.h
M src/fsm.c
2 files changed, 39 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/15833/1

diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 269befa..8a1999f 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -123,6 +123,7 @@
 void osmo_fsm_log_timeouts(bool log_timeouts);
 void osmo_fsm_term_safely(bool term_safely);
 void osmo_fsm_set_dealloc_ctx(void *ctx);
+void osmo_fsm_set_term_stops_actions(bool term_stops_actions);
 
 /*! Log using FSM instance's context, on explicit logging subsystem and level.
  * \param fi  An osmo_fsm_inst.
diff --git a/src/fsm.c b/src/fsm.c
index 6aad37a..9c83647 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -104,6 +104,8 @@
 	void *collect_ctx;
 	/*! See osmo_fsm_set_dealloc_ctx() */
 	void *fsm_dealloc_ctx;
+	/*! If true, refuse events and state changes on terminating FSM instances. */
+	bool term_stops_actions;
 } fsm_term_safely;
 
 /*! Internal call to free an FSM instance, which redirects to the context set by osmo_fsm_set_dealloc_ctx() if any.
@@ -200,6 +202,27 @@
 	fsm_term_safely.fsm_dealloc_ctx = ctx;
 }
 
+/*! For FSM instances that are terminating or already terminated, refuse to dispatch events or make state changes.
+ *
+ * Where multiple FSM instances are collaborating, a terminating FSM instance often causes events to be dispatched back
+ * to itself, or causes state changes in FSM instances that are already terminating. That is hard to avoid when each FSM
+ * instance could be a cause of failure, and wants to notify others, which in turn may choose to terminate.
+ *
+ * Another use case: any function that dispatches events or state changes to more than one FSM instance must be sure
+ * that after the first event dispatch, the second FSM instance is in fact still allocated. Furthermore, if the second
+ * FSM instance *has* terminated from the first dispatch, this often means that no more actions should be taken. That
+ * could be done by an explicit check for fsm->proc.terminating, but a more general solution is to do enable this check
+ * internally in fsm.c, with this function.
+ *
+ * The default behavior to still dispatch events and state changes is kept for legacy reasons.
+ *
+ * \param[in] term_stops_actions  If true, refuse actions on terminated FSM instances.
+ */
+void osmo_fsm_set_term_stops_actions(bool term_stops_actions)
+{
+	fsm_term_safely.term_stops_actions = term_stops_actions;
+}
+
 /*! talloc_free() the given object immediately, or once ongoing FSM terminations are done.
  *
  * If an FSM deallocation cascade is ongoing, talloc_steal() the given talloc_object into the talloc context that is
@@ -630,6 +653,13 @@
 	const struct osmo_fsm_state *st = &fsm->states[fi->state];
 	struct timeval remaining;
 
+	if (fsm_term_safely.term_stops_actions && fi->proc.terminating) {
+		LOGPFSMSRC(fi, file, line,
+			   "FSM instance already terminating, not changing state to %s\n",
+			   osmo_fsm_state_name(fsm, new_state));
+		return -EINVAL;
+	}
+
 	/* validate if new_state is a valid state */
 	if (!(st->out_state_mask & (1 << new_state))) {
 		LOGPFSMLSRC(fi, LOGL_ERROR, file, line,
@@ -840,6 +870,14 @@
 	}
 
 	fsm = fi->fsm;
+
+	if (fsm_term_safely.term_stops_actions && fi->proc.terminating) {
+		LOGPFSMSRC(fi, file, line,
+			   "FSM instance already terminating, not dispatching event %s\n",
+			   osmo_fsm_event_name(fsm, event));
+		return -EINVAL;
+	}
+
 	OSMO_ASSERT(fi->state < fsm->num_states);
 	fs = &fi->fsm->states[fi->state];
 

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15833
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I0adc13a1a998e953b6c850efa2761350dd07e03a
Gerrit-Change-Number: 15833
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191023/8ff44094/attachment.htm>


More information about the gerrit-log mailing list