[MERGED] libosmocore[master]: fsm: allow graceful exit on FSM termination
Harald Welte has submitted this change and it was merged.
Change subject: fsm: allow graceful exit on FSM termination
..
fsm: allow graceful exit on FSM termination
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
---
M TODO-RELEASE
M include/osmocom/core/fsm.h
M src/fsm.c
3 files changed, 16 insertions(+), 2 deletions(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 782ba19..928b18d 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
core msgb_queue_free() add inline func to msgb.h
coding gsm0503_rach_ext-encode() add func to gsm0503_coding.h
codec ecu.c / ecu.h implement ECU for FR (Error
Concealment Unit)
+fsmfsmc / fsm.hadded callback for graceful
exit => ABI changed
\ No newline at end of file
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index bbfe312..2c2a996 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -78,6 +78,8 @@
int log_subsys;
/*! human-readable names of events */
const struct value_string *event_names;
+ /*! graceful exit function, called at the beginning of termination */
+ void (*pre_term)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause);
};
/*! a single instanceof an osmocom finite state machine */
diff --git a/src/fsm.c b/src/fsm.c
index a127362..176aa8a 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -298,7 +298,11 @@
/*! unlink child FSM from its parent FSM.
* \param[in] fi Descriptor of the child FSM to unlink.
- * \param[in] ctx New talloc context */
+ * \param[in] ctx New talloc context
+ *
+ * Never call this function from the cleanup callback, because at that time
+ * the child FSMs will already be terminated. If unlinking should be performed
+ * on FSM termination, use the grace callback instead. */
void osmo_fsm_inst_unlink_parent(struct osmo_fsm_inst *fi, void *ctx)
{
if (fi->proc.parent) {
@@ -312,7 +316,10 @@
/*! change parent instance of an FSM.
* \param[in] fi Descriptor of the to-be-allocated FSM.
* \param[in] new_parent New parent FSM instance.
- * \param[in] new_parent_term_event Event to be sent to parent when
terminating. */
+ * \param[in] new_parent_term_event Event to be sent to parent when
terminating.
+ *
+ * Never call this function from the cleanup callback!
+ * (see also osmo_fsm_inst_unlink_parent()).*/
void osmo_fsm_inst_change_parent(struct osmo_fsm_inst *fi,
struct osmo_fsm_inst *new_parent,
uint32_t new_parent_term_event)
@@ -528,6 +535,10 @@
LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
osmo_fsm_term_cause_name(cause));
+ /* graceful exit (optional) */
+ if (fi->fsm->pre_term)
+ fi->fsm->pre_term(fi, cause);
+
_osmo_fsm_inst_term_children(fi, OSMO_FSM_TERM_PARENT, NULL,
file, line);
--
To view, visit https://gerrit.osmocom.org/6452
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Gerrit-PatchSet: 4
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
libosmocore[master]: fsm: allow graceful exit on FSM termination
Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/6452 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb Gerrit-PatchSet: 3 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-HasComments: No
[PATCH] libosmocore[master]: fsm: allow graceful exit on FSM termination
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/6452
to look at the new patch set (#3).
fsm: allow graceful exit on FSM termination
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
---
M TODO-RELEASE
M include/osmocom/core/fsm.h
M src/fsm.c
3 files changed, 16 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/6452/3
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 782ba19..928b18d 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
core msgb_queue_free() add inline func to msgb.h
coding gsm0503_rach_ext-encode() add func to gsm0503_coding.h
codec ecu.c / ecu.h implement ECU for FR (Error
Concealment Unit)
+fsmfsmc / fsm.hadded callback for graceful
exit => ABI changed
\ No newline at end of file
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 8f550d1..ddc7b62 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -78,6 +78,8 @@
int log_subsys;
/*! human-readable names of events */
const struct value_string *event_names;
+ /*! graceful exit function, called at the beginning of termination */
+ void (*pre_term)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause);
};
/*! a single instanceof an osmocom finite state machine */
diff --git a/src/fsm.c b/src/fsm.c
index d8751c9..68bac7f 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -279,7 +279,11 @@
/*! unlink child FSM from its parent FSM.
* \param[in] fi Descriptor of the child FSM to unlink.
- * \param[in] ctx New talloc context */
+ * \param[in] ctx New talloc context
+ *
+ * Never call this function from the cleanup callback, because at that time
+ * the child FSMs will already be terminated. If unlinking should be performed
+ * on FSM termination, use the grace callback instead. */
void osmo_fsm_inst_unlink_parent(struct osmo_fsm_inst *fi, void *ctx)
{
if (fi->proc.parent) {
@@ -293,7 +297,10 @@
/*! change parent instance of an FSM.
* \param[in] fi Descriptor of the to-be-allocated FSM.
* \param[in] new_parent New parent FSM instance.
- * \param[in] new_parent_term_event Event to be sent to parent when
terminating. */
+ * \param[in] new_parent_term_event Event to be sent to parent when
terminating.
+ *
+ * Never call this function from the cleanup callback!
+ * (see also osmo_fsm_inst_unlink_parent()).*/
void osmo_fsm_inst_change_parent(struct osmo_fsm_inst *fi,
struct osmo_fsm_inst *new_parent,
uint32_t new_parent_term_event)
@@ -509,6 +516,10 @@
LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
osmo_fsm_term_cause_name(cause));
+ /* graceful exit (optional) */
+ if (fi->fsm->pre_term)
+ fi->fsm->pre_term(fi, cause);
+
_osmo_fsm_inst_term_children(fi, OSMO_FSM_TERM_PARENT, NULL,
file, line);
--
To view, visit https://gerrit.osmocom.org/6452
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Gerrit-PatchSet: 3
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
libosmocore[master]: fsm: allow graceful exit on FSM termination
Patch Set 2: (1 comment) https://gerrit.osmocom.org/#/c/6452/2/include/osmocom/core/fsm.h File include/osmocom/core/fsm.h: Line 73:void (*pre_term)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cause); we should add new members at the end of the struct. I think this will make any ABI incompatibility between old-lib/new-app or vice-versa more likely to cause an immediate error (until libversion is bumped) -- To view, visit https://gerrit.osmocom.org/6452 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb Gerrit-PatchSet: 2 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-HasComments: Yes
libosmocore[master]: fsm: allow graceful exit on FSM termination
Patch Set 2: > I think we can introduce this. However, I'm not happy with the > naming. First, "grace" is different from "grace period" or > "graceful" (think of "holy grace", ...). Second, I think it's not > very indicative/explanatory of what is actually happening. > Withouht thinking too much about it, I think "pre_term" or even > "term" might be a beter name. After all, it's called from > "osmo_fsm_inst_term". So first we call the termination code, then > we terminate children and finally we cleanup, just before the free. Yes, I think pre_term makes more sense. I have changed it now. -- To view, visit https://gerrit.osmocom.org/6452 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb Gerrit-PatchSet: 2 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-HasComments: No
[PATCH] libosmocore[master]: fsm: allow graceful exit on FSM termination
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/6452
to look at the new patch set (#2).
fsm: allow graceful exit on FSM termination
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
---
M TODO-RELEASE
M include/osmocom/core/fsm.h
M src/fsm.c
3 files changed, 16 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/6452/2
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 782ba19..928b18d 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
core msgb_queue_free() add inline func to msgb.h
coding gsm0503_rach_ext-encode() add func to gsm0503_coding.h
codec ecu.c / ecu.h implement ECU for FR (Error
Concealment Unit)
+fsmfsmc / fsm.hadded callback for graceful
exit => ABI changed
\ No newline at end of file
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 8f550d1..d5349dc 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -69,6 +69,8 @@
uint32_t allstate_event_mask;
/*! function pointer to be called for allstate events */
void (*allstate_action)(struct osmo_fsm_inst *fi, uint32_t event, void
*data);
+ /*! graceful exit function, called at the beginning of termination */
+ void (*pre_term)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause);
/*! clean-up function, called during termination */
void (*cleanup)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause);
/*! timer call-back for states with time-out.
diff --git a/src/fsm.c b/src/fsm.c
index d8751c9..68bac7f 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -279,7 +279,11 @@
/*! unlink child FSM from its parent FSM.
* \param[in] fi Descriptor of the child FSM to unlink.
- * \param[in] ctx New talloc context */
+ * \param[in] ctx New talloc context
+ *
+ * Never call this function from the cleanup callback, because at that time
+ * the child FSMs will already be terminated. If unlinking should be performed
+ * on FSM termination, use the grace callback instead. */
void osmo_fsm_inst_unlink_parent(struct osmo_fsm_inst *fi, void *ctx)
{
if (fi->proc.parent) {
@@ -293,7 +297,10 @@
/*! change parent instance of an FSM.
* \param[in] fi Descriptor of the to-be-allocated FSM.
* \param[in] new_parent New parent FSM instance.
- * \param[in] new_parent_term_event Event to be sent to parent when
terminating. */
+ * \param[in] new_parent_term_event Event to be sent to parent when
terminating.
+ *
+ * Never call this function from the cleanup callback!
+ * (see also osmo_fsm_inst_unlink_parent()).*/
void osmo_fsm_inst_change_parent(struct osmo_fsm_inst *fi,
struct osmo_fsm_inst *new_parent,
uint32_t new_parent_term_event)
@@ -509,6 +516,10 @@
LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
osmo_fsm_term_cause_name(cause));
+ /* graceful exit (optional) */
+ if (fi->fsm->pre_term)
+ fi->fsm->pre_term(fi, cause);
+
_osmo_fsm_inst_term_children(fi, OSMO_FSM_TERM_PARENT, NULL,
file, line);
--
To view, visit https://gerrit.osmocom.org/6452
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder
libosmocore[master]: fsm: allow graceful exit on FSM termination
Patch Set 1: I think we can introduce this. However, I'm not happy with the naming. First, "grace" is different from "grace period" or "graceful" (think of "holy grace", ...). Second, I think it's not very indicative/explanatory of what is actually happening. Withouht thinking too much about it, I think "pre_term" or even "term" might be a beter name. After all, it's called from "osmo_fsm_inst_term". So first we call the termination code, then we terminate children and finally we cleanup, just before the free. -- To view, visit https://gerrit.osmocom.org/6452 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb Gerrit-PatchSet: 1 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: dexter Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: No
[PATCH] libosmocore[master]: fsm: allow graceful exit on FSM termination
Review at https://gerrit.osmocom.org/6452
fsm: allow graceful exit on FSM termination
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
---
M TODO-RELEASE
M include/osmocom/core/fsm.h
M src/fsm.c
3 files changed, 16 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/6452/1
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 782ba19..928b18d 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -10,3 +10,4 @@
core msgb_queue_free() add inline func to msgb.h
coding gsm0503_rach_ext-encode() add func to gsm0503_coding.h
codec ecu.c / ecu.h implement ECU for FR (Error
Concealment Unit)
+fsmfsmc / fsm.hadded callback for graceful
exit => ABI changed
\ No newline at end of file
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 8f550d1..1b4dcee 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -69,6 +69,8 @@
uint32_t allstate_event_mask;
/*! function pointer to be called for allstate events */
void (*allstate_action)(struct osmo_fsm_inst *fi, uint32_t event, void
*data);
+ /*! graceful exit function, called at the beginning of termination */
+ void (*grace)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cause);
/*! clean-up function, called during termination */
void (*cleanup)(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause
cause);
/*! timer call-back for states with time-out.
diff --git a/src/fsm.c b/src/fsm.c
index d8751c9..859f7a9 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -279,7 +279,11 @@
/*! unlink child FSM from its parent FSM.
* \param[in] fi Descriptor of the child FSM to unlink.
- * \param[in] ctx New talloc context */
+ * \param[in] ctx New talloc context
+ *
+ * Never call this function from the cleanup callback, because at that time
+ * the child FSMs will already be terminated. If unlinking should be performed
+ * on FSM termination, use the grace callback instead. */
void osmo_fsm_inst_unlink_parent(struct osmo_fsm_inst *fi, void *ctx)
{
if (fi->proc.parent) {
@@ -293,7 +297,10 @@
/*! change parent instance of an FSM.
* \param[in] fi Descriptor of the to-be-allocated FSM.
* \param[in] new_parent New parent FSM instance.
- * \param[in] new_parent_term_event Event to be sent to parent when
terminating. */
+ * \param[in] new_parent_term_event Event to be sent to parent when
terminating.
+ *
+ * Never call this function from the cleanup callback!
+ * (see also osmo_fsm_inst_unlink_parent()).*/
void osmo_fsm_inst_change_parent(struct osmo_fsm_inst *fi,
struct osmo_fsm_inst *new_parent,
uint32_t new_parent_term_event)
@@ -509,6 +516,10 @@
LOGPFSMSRC(fi, file, line, "Terminating (cause = %s)\n",
osmo_fsm_term_cause_name(cause));
+ /* graceful exit (optional) */
+ if (fi->fsm->grace)
+ fi->fsm->grace(fi, cause);
+
_osmo_fsm_inst_term_children(fi, OSMO_FSM_TERM_PARENT, NULL,
file, line);
--
To view, visit https://gerrit.osmocom.org/6452
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter
