[MERGED] libosmocore[master]: fsm: term: safer iteration to remove all child FSMs

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: fsm: term: safer iteration to remove all child FSMs
......................................................................


fsm: term: safer iteration to remove all child FSMs

When terminating child FSMs, restart iteration after every child, to make
sure that we don't terminate a child twice. Terminating one child may emit
events that in turn terminates other children.

I created this patch because at first it looked like the cause of a bug,
which turned out not to be the case. So I have no actual use case of this
situation, but it does generally make sense to me, so submitting this.

Change-Id: I00990b47e42eeb43707a9a42abcd9df52fe5f483
---
M src/fsm.c
1 file changed, 20 insertions(+), 4 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/fsm.c b/src/fsm.c
index 6717538..16699fa 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -427,17 +427,33 @@
 			 enum osmo_fsm_term_cause cause, void *data,
 			 const char *file, int line)
 {
-	struct osmo_fsm_inst *child, *child2;
+	struct osmo_fsm_inst *first_child, *last_seen_first_child;
 	struct osmo_fsm_inst *parent = fi->proc.parent;
 	uint32_t parent_term_event = fi->proc.parent_term_event;
 
 	LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
 		   osmo_fsm_term_cause_name(cause));
 
-	/* iterate over all children */
-	llist_for_each_entry_safe(child, child2, &fi->proc.children, proc.child) {
+	/* iterate over all children, starting from the beginning every time:
+	 * terminating an FSM may emit events that cause other FSMs to also
+	 * terminate and remove themselves from this list. */
+	last_seen_first_child = NULL;
+	while (!llist_empty(&fi->proc.children)) {
+		first_child = llist_entry(fi->proc.children.next,
+					  typeof(*first_child),
+					  proc.child);
+
+		/* paranoia: do not loop forever */
+		if (first_child == last_seen_first_child) {
+			LOGPFSMLSRC(fi, LOGL_ERROR, file, line,
+				    "Internal error while terminating child"
+				    " FSMs: a child FSM is stuck\n");
+			break;
+		}
+		last_seen_first_child = first_child;
+
 		/* terminate child */
-		_osmo_fsm_inst_term(child, OSMO_FSM_TERM_PARENT, NULL,
+		_osmo_fsm_inst_term(first_child, OSMO_FSM_TERM_PARENT, NULL,
 				    file, line);
 	}
 

-- 
To view, visit https://gerrit.osmocom.org/1457
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I00990b47e42eeb43707a9a42abcd9df52fe5f483
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder