https://osmocom.org/https://osmocom.org/favicon.ico?16647414092022-08-01T14:42:30ZOpen Source Mobile Communicationslibosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245412022-08-01T14:42:30Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>no. osmo_fsm_inst_state_chg(timeout_secs == 0) is exactly intended to change the state and not set any timeout.<br />There is no use case for changing the state and immediately timing out.<br />Immediate actions are either in the onenter() cb or directly osmo_fsm_inst_term().</p>
<p>Anything here i am misunderstanding? otherwise please reject this issue.</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245422022-08-01T15:14:47Zpespin
<ul></ul><p>There's a difference between "immediately", aka "synchronously" doing an action, and delaying an action 0 seconds, aka "asynchronously" doing an action.</p>
<p>Think of a function calculating a timer timeout value dynamically, and finding out it is late, so setting timeout=0. In this case, the timeout will be unexpectedly disabled.</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245432022-08-01T15:29:51Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>ah you mean the use case "defer" -- take an action in the next (or so) select loop.<br />i see that as a new feature...</p>
<p>i can understand the notion that extrapolating a smaller and smaller delay down to zero would end up becoming "immediate" timeout,<br />but the state change from the start was intended to not have any timeout on zero.<br />i think that is fine, because IMHO a defer is orthogonal to state change; i think i would design it as an event dispatch instead?</p>
<pre><code>osmo_fsm_inst_dispatch_deferred(fi, EVENT);</code></pre> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245442022-08-01T15:38:03Zpespin
<ul></ul><p>"deferring" an event is one use case, but the other one which is not really deferring is setting up a dynamically-calculated timeout value when changing state.<br />It's totally common to, when calculating time diffs, set a timeout value to 0 when it's too late. In this case, the FSM will just silently break with an unexpected behavior.</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245462022-08-01T16:11:30Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>i find the behaviour expected.<br />special case when you hit zero and call the desired action immediately?</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245472022-08-01T16:13:15Zpespin
<ul></ul><p><a class="user active" href="https://osmocom.org/users/91">neels</a> we are talking about changing state, not dispatching an event here.<br />The state change is always instantaneous/synchonous, what we talk is about the timeout being disabled if 0 is set.</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245652022-08-03T18:45:25Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>I'm sure you will find a solution.</p>
<p>API wise my humble opinion is that state_chg(timeout_secs=0) meaning "never<br />time out" is definitely not a bug.</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245672022-08-04T08:12:09Zlaforge
<ul></ul><p>Sorry for being slow to respond. I've been distracted by various emergencies in recent days.</p>
<p>As far as I recall: In general, I have to say that the <em>timeout == 0</em> for "no timeout" was a conscious choice when originally wrigin osmo_fsm. As far as I recalled, the initial code had no timeout support at all, and then I thought it's useful to have an optional timeout to avoid having to call two functions for state change + start of timeout.</p>
<p>At that time, I would never have thought of a dynamic timeout, but only about static, compiled-in timeout values. Those can never go to 0 by accident.</p>
<p>I understand the failure mode pespin is describing, but I still think it is not a valid use case to "abuse" the timeout=0 for some deferred execution.</p>
<p>So IMHO, if we should fix something, then it is whatever code that generates dynamic (or user-modifiable) timeout values: It should never return "0".</p> libosmocore - Bug #5638: osmo_fsm: osmo_fsm_inst_state_chg() using timeout_secs=0 instead of -1 as "no-timeout"https://osmocom.org/issues/5638?journal_id=245752022-08-04T09:31:59Zpespin
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Rejected</i></li></ul>