Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling Timer::start or Timer::stop in the same timer's callback leads to unexpected behavior #1532

Closed
tay64 opened this issue Aug 25, 2022 · 0 comments · Fixed by #1533
Closed

Comments

@tay64
Copy link
Contributor

tay64 commented Aug 25, 2022

Description

Assume this pseudocode:

let the_timer = Timer::default();
the_timer.start(some_mode, some_interval, some_callback);

Undesired behaviors

  1. If some_callback calls the_timer.start(new_mode, new_interval, new_callback), the effect will be the_timer running with new_mode and new_interval, but the old some_callback.
    Under some circumstances this leads to unobvious effects, like a single-shot timer seemingly turning into a periodic timer (see Steps to reproduce below for an example).

  2. If some_mode is Repeated and some_callback calls the_timer.stop(), the timer won't stop.

Analysis

This is caused by the order of operations in internal/core/timers.rs :: maybe_activate_timers(); specifically, by the fact that the callback is invoked in the middle of the timer state update. As a consequence, if the callback itself modifies the timer state via the timer API, part of the modifications will be overwritten by maybe_activate_timers().

I have a possible fix. I will clean it up a bit and submit a PR for your consideration. (EDIT: PR #1533)

Steps to reproduce

This is not the smallest repro example, just the one I came up with while distilling the problem I encountered in my project. I decided to include it here because it demonstrates a surprising behavior that at a first glance looks as if a SingleShot timer turns into a Repeated (the real explanation is that every time the first callback is invoked, it restarts the timer with the second callback, but the second callback gets overwritten with the first callback again).

  1. Start with cargo generate --git https://github.com/slint-ui/slint-rust-template.
  2. Apply this diff:
    diff file
    modified main.rs
  3. cargo run
  4. Click on the button in the example app.

Expected:

1661460594.8351786s  start_timer
1661460595.8354115s  timer_callback_start_anew
1661460596.0354897s  timer_callback_final
(end of activity)

Observed:

1661460293.2738762s  start_timer
1661460294.2737572s  timer_callback_start_anew
1661460294.4736632s  timer_callback_start_anew
1661460294.6743098s  timer_callback_start_anew
1661460294.8737588s  timer_callback_start_anew
1661460295.0736991s  timer_callback_start_anew
...

Note that starting with the 3rd line the timestamp delta is 200ms, i.e. the new interval took effect, but the wrong callback is invoked.

@tay64 tay64 changed the title Calling Timer::start in the same timer's callback leads to unexpected behavior Calling Timer::start or Timer::stop in the same timer's callback leads to unexpected behavior Aug 25, 2022
tay64 added a commit to tay64/slint that referenced this issue Aug 25, 2022
…lback

Before the change:

- calling the_timer.start(...) in the_timer's callback resulted in the_timer
keeping the old callback;

- calling the_timer.stop() in the_timer's callback was ignored.
tay64 added a commit to tay64/slint that referenced this issue Aug 25, 2022
Before the change:

- calling the_timer.start(...) in the_timer's callback resulted in the_timer
keeping the old callback;

- calling the_timer.stop() in the_timer's callback was ignored.
tay64 added a commit to tay64/slint that referenced this issue Aug 25, 2022
Before the change:

- calling the_timer.start(...) in the_timer's callback resulted in the_timer
keeping the old callback;

- calling the_timer.stop() in the_timer's callback was ignored.
ogoffart pushed a commit that referenced this issue Aug 26, 2022
Before the change:

- calling the_timer.start(...) in the_timer's callback resulted in the_timer
keeping the old callback;

- calling the_timer.stop() in the_timer's callback was ignored.
ogoffart added a commit that referenced this issue Aug 26, 2022
Tests that timer fires, but also that stopping them within a timer event work
(Test for #1532 and #1533)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant