diff options
| author | Neels Hofmeyr <nhofmeyr@sysmocom.de> | 2016-12-20 12:05:19 +0100 | 
|---|---|---|
| committer | Neels Hofmeyr <nhofmeyr@sysmocom.de> | 2016-12-20 12:39:32 +0100 | 
| commit | 06ac9b40ed600cd36e1c0a158eb8f2ca01b1941f (patch) | |
| tree | 6b324f04d88482bd3a7deb47922b567f9d69fc55 | |
| parent | 42b59c1bf080a5ae785f576ba749afdf3bde598d (diff) | |
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
| -rw-r--r-- | src/fsm.c | 24 | 
1 files changed, 20 insertions, 4 deletions
| @@ -427,17 +427,33 @@ void _osmo_fsm_inst_term(struct osmo_fsm_inst *fi,  			 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);  	} | 
