xiaoxiang781216 commented on PR #17573: URL: https://github.com/apache/nuttx/pull/17573#issuecomment-3710690437
> 4. Even reverting to the original implementation still violates the invariant because **`expired` is not monotonically increasing and cannot be used as a version**. It is possible for the old and new timers to have the same `expired` time, allowing a cancelled timer to modify the new timer. > > ```c > // executing callback... > if (period > 0 && hrtimer->expired == expired) > ``` > > It violates the ownership invariant when: > > 1. If a periodic timer `hrtimer->expired = t1`, `hrtimer->func = period_func` is executing. > 2. While the callback is executing, the timer is cancleled and a new timer is started with `hrtimer->expired = t1`, `hrtimer->func = oneshot_func`. > 3. When the cancelled `period_func` finishes, while `oneshot_func` is still executing, the cancelled `period_func` overwrites the newly set oneshot timer, causing an error. but we just provide hrtimer_init to change `hrtimer->func`, which mean the `func` can't be changed after initialization. > > Even if we add a check for `func` as suggested by @wangchdo , problems remain: > > ```c > // executing callback... > if (period > 0 && hrtimer->expired == expired && hrtimer->func == func) > ``` > > It violates the ownership invariant when: > > 1. If a periodic timer `hrtimer->expired = t1`, `hrtimer->func = period_func` is executing, and `period_func` returns the next delay `p1`. > 2. At time `t1`, while the hrtimer callback is executing, the periodic timer is cancelled, and a new periodic timer is set to trigger immediately with `hrtimer->expired = t1`, `hrtimer->func = period_func`, which returns the next delay `p2`. > 3. When the cancelled previous callback finishes, while the new callback is still executing, all version checks pass, and the next timer is set to `hrtimer->expired = t1 + p1`, overwriting the new timer (the expected next expired time should be `hrtimer->expired = t1 + p2`). in this case, the timeout will be set to `hrtimer->expired = t1 + p1`, but overwrite by `hrtimer->expired = t1 + p2`. I think it's right behaviour the last timeout win the competition. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
