-
Notifications
You must be signed in to change notification settings - Fork 604
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
assertion failed: !timer.being_activated
#6187
Comments
Is it possible perhaps that the timer was dropped in its own timeout callback? (In any case, this is a bug, but we need to find a way to reproduce it :-/ |
(cc @ogoffart - would this area be... acceptable?) |
I use the
I've just repeated the process many times, but couldn't reproduce the panic. It may be a heisenbug. |
I thought it happens if, for example, one call let t1 = Timer::default();
t1.start(TimerMode::Repeated, Duration::from_millis(1), move || {
slint::platform::update_timers_and_animations();
}); and we get another panic ("Recursion in timer code" line 257) so that's not the same. I would love to see a reproducable test case. or even just a backtrace. |
After always running my app with
|
|
I don't understand how this is possible :( This is the offending mutable indexing call: let timers_to_process = core::mem::take(&mut timers.borrow_mut().active_timers);
{
let mut timers = timers.borrow_mut();
for active_timer in &timers_to_process {
let timer = &mut timers.timers[active_timer.id];
assert!(!timer.being_activated);
timer.being_activated = true;
}
} The only situation that I can imagine this panicking is if the same timer id exists twice in |
Is this quote from
Could you maybe add debug-build-only code at different places that temporarily does many more checks of all kinds? (Or, if you think this is better, even behind a secret feature called |
Yeah, I was also contemplating switching to generational area like we use it elsewhere. But this particular situation, use after removal is supposed to be already handled with the
That could be done. How easily can you trigger the bug in your application? |
Could you try this in your [patch.crates-io]
slint = { git = "https://github.com/slint-ui/slint", branch = "simon/timer-debug" }
slint-build = { git = "https://github.com/slint-ui/slint", , branch = "simon/timer-debug" } |
You have a double-comma in the TOML table (
I'm usually productively running the app's debug build one to two times a day, and I told you of every panic in this context. I never got one of these panics through short unproductive app usage, even though I tried with repeated actions. |
Despite your additions, the same
|
Thanks for testing. I've found three bugs in the code, two fixed in 2f9cea5 and one in 787e13b . I was able to reproduce one of the invalid key panics, but oddly not the one you ran into. Perhaps however that was the consequence of incorrect id re-use, that 787e13b would fix. I'd be very grateful if you could test the current |
The same
|
I encountered another bug where a timer ID wasn't found:
fn deactivate_timer(&mut self, id: usize) {
let mut i = 0;
while i < self.active_timers.len() {
if self.active_timers[i].id == id {
self.active_timers.remove(i);
self.timers[id].running = false;
break;
} else {
i += 1;
}
}
} When debugging stepwise, I found out the loop in |
Yes indeed, I can see how this could happen. This is - in essence - the same as the other two issues I noticed: during timer callback invocation the active_timers data structure is in an inconsistent state. Trying to stop a future timer from within an activated timer callback triggers this, as stop() fails to locate the timer in the just emptied active_timers. I'll fix this on Monday. Before invoking any callbacks we must make sure that the data structures are in a consistent ready-for-anything state. |
I pushed a fix for this stop() issue to the branch. Could you try and see if that helps? I think that this could also help with the invalid key issue. |
I was able to branch out app states that allow me to trigger all problems with the unpatched Slint code ( With your branch in its current state:
|
Looks like we got all of the issues then. Thank you so much for testing and the quick responses! I'll prepare a PR, after doing some cleanup. |
- When stopping a running but not yet expired timer from within the activation callback of an expired timer, the stopping would fail to find the timer in the active_timers list because it was built on the fly. Similarly, restarting a future timer form within the same kind of callback would end up registering the active timer twice. Fix this by doing all the active_timer re-setup before activating any callbacks, so that by the time of activation the data structure is in a good state. - When two timers expire at the same time and from the callback of the first, the second timer would be dropped/deleted, the timer's `being_activated` was overwritten and the timer was deleted too early, causing a panic. Fix this by preserving the removal state. Fixes #6187 Fixes #6505
I use Rust on Windows 10 with Slint v1.7.2.
For the first time, I encountered the following panic, which didn't happen in many cases that should be identical from my point of view:
I don't have much information about it. But I think it could have happened in the context of dropping the
Timer
that I hold in myApp
struct by setting the respective struct field toNone
. In my app, I often dropTimer
s like this without stopping them first.The text was updated successfully, but these errors were encountered: