From b1bbe24c09c0f508d7bc916f08b1ed4d09986df5 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Sat, 5 Oct 2019 05:13:23 +0200 Subject: fsm: refuse state chg and events after term Refuse state changes and event dispatch for FSM instances that are already terminating. It is assumed that refusing state changes and events after FSM termination is seen as the sane expected behavior, hence this change in behavior is merged without being configurable. There is no fallout in current Osmocom code trees. fsm_dealloc_test needs a changed expected output, since it is explicitly creating complex FSM structures that terminate. Currently no other C test in Osmocom code needs adjusting. 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: Ia81a0892f710db86bd977462730b69f0dcc78f8c --- src/fsm.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'src') diff --git a/src/fsm.c b/src/fsm.c index 6aad37af..1e8909ec 100644 --- a/src/fsm.c +++ b/src/fsm.c @@ -630,6 +630,13 @@ static int state_chg(struct osmo_fsm_inst *fi, uint32_t new_state, const struct osmo_fsm_state *st = &fsm->states[fi->state]; struct timeval remaining; + if (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 +847,14 @@ int _osmo_fsm_inst_dispatch(struct osmo_fsm_inst *fi, uint32_t event, void *data } fsm = fi->fsm; + + if (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]; -- cgit v1.2.3