termio: don't start scroll timer if its already active (#9195)

This might fix #9191, but since I don't have a reproduction I can't be
sure. In any case, this is a bad bug that should be fixed.

The issue is that we weren't checking our scroll timer completion state.
This meant that if `start_scroll_timer` was called multiple times within
a single loop tick, we'd enqueue our completion multiple times, leading
to various undefined behaviors.

If we don't enqueue anything else in the interim, this is safe by
chance. But if we enqueue something else, then we'd hit a queue
assertion failure and honestly I'm not totally sure what would happen.

I wasn't able to trigger the "bad" case, but I was able to trigger the
benign case very easily. Our other timers such as the renderer cursor
timer already have this protection.

Let's fix this and continue looking...
pull/9200/head
Mitchell Hashimoto 2025-10-13 20:44:32 -07:00 committed by GitHub
parent 5462553741
commit 17a20e5b1c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 17 additions and 9 deletions

View File

@ -471,15 +471,23 @@ fn stopCallback(
fn startScrollTimer(self: *Thread, cb: *CallbackData) void {
self.scroll_active = true;
// Start the timer which loops
self.scroll.run(
&self.loop,
&self.scroll_c,
selection_scroll_ms,
CallbackData,
cb,
selectionScrollCallback,
);
switch (self.scroll_c.state()) {
// If it is already active, e.g. startScrollTimer is called multiple
// times, then we just return. We can't simply check `scroll_active`
// because its possible that `stopScrollTimer` was called but there
// was no loop tick between then and now to halt out completion.
.active => return,
// If the completion is not active then we need to start it.
.dead => self.scroll.run(
&self.loop,
&self.scroll_c,
selection_scroll_ms,
CallbackData,
cb,
selectionScrollCallback,
),
}
}
fn stopScrollTimer(self: *Thread) void {