fix(pet): stop the hover watcher with the window it watches - #711
Open
Adam-Dalloul wants to merge 1 commit into
Open
fix(pet): stop the hover watcher with the window it watches#711Adam-Dalloul wants to merge 1 commit into
Adam-Dalloul wants to merge 1 commit into
Conversation
`spawn_pet_hover_watcher` polls the windowing layer on an 80 ms timer for as long as the pet window is open: `cursor_position` every tick, plus `outer_position` and `outer_size` every fifth. Each of those is a synchronous request/reply round trip to the main thread, answered inside tauri's event loop, and the loop only stops once a tick happens to find the pet missing from the window map. Nothing else in the process polls the windowing layer on a timer, and left running it is about 1.5M round trips a day at an idle machine. Two ways that outlives what it watches today. On the quit path, `RunEvent::ExitRequested` blocks the main thread on the web-server stop and on the ACP disconnects. Every watcher tick during that window posts a request only that blocked thread can answer, and blocks a tokio worker waiting for it, while the event loop is on its way down. Window close has the same shape in miniature: the watcher keeps round-tripping at a window between `CloseRequested` and the point where it leaves the window map. A second watcher can also exist. `open_pet_window` early-returns only while `get_webview_window` still answers, so a close followed by a re-open inside one tick builds a new window before the old loop sees the gap. The old loop then finds the new window, never exits, and two watchers poll in parallel while fighting over the single `PET_HOVER_WAS_INSIDE` flag. So the watcher now holds a `CancellationToken` in a process-global slot. Installing a watcher cancels whatever it replaced, which is the at-most-one invariant; `stop_pet_hover_watcher` installs `None`, which is the stop. The loop selects on the token `biased` ahead of the tick, so a stop lands before the next round trip rather than one tick after it. Both branches are cancel safe, so losing either drops no tick and no cancellation. `lib.rs` stops the watcher from the pet window's close/destroy branch and again at the top of `ExitRequested`, before anything there blocks the main thread; a watcher whose window is already gone gets no close event, so the second call is not redundant. Behavior is unchanged while the pet window is open: same interval, same bounds cache, same enter/leave events, on every platform. Not gated to macOS despite the doc comment's macOS-shaped rationale, because `PetWindow.tsx` carries no DOM hover handler of its own; these events are the only thing that drives the waving animation anywhere, so a gate would silently drop hover-waving on Windows and Linux.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #703, on the one idle-crash candidate that audit deliberately left alone.
The crash it is aimed at:
codeg.exe0.30.6.0 aborted with0xc0000409(WER bucket BEX64) after sitting idle for about three days, with nothing in the log. Nobody was at the keyboard, so whatever ran had to be on a timer.spawn_pet_hover_watcheris the only thing in the process that polls the windowing layer on a timer. It runs an 80 ms loop for as long as the pet window is open:cursor_positionevery tick,outer_positionandouter_sizeevery fifth. Each of those is a synchronous request/reply round trip answered on the main thread inside tauri's event loop, so it is roughly 1.5M round trips a day at an idle machine, and the loop only stops once a tick happens to find the pet gone from the window map.That is late in two places. On quit,
ExitRequestedblocks the main thread on the web-server stop and the ACP disconnects, and every watcher tick during that window posts a request only that blocked thread can answer while the event loop is already coming down. A second watcher can also exist:open_pet_windowearly-returns only whileget_webview_windowstill answers, so a close followed by a re-open inside one tick builds a new window before the old loop sees the gap, and the old loop then adopts the new window, never exits, and races the first over the shared hover flag.I want to be clear that this is the leading suspect and not a proven cause. There is no log line naming the failure, the crash was not reproduced, and the fix is justified on its own terms: a poll loop should not outlive the window it watches or duplicate itself.
What changes: the watcher holds a
CancellationTokenin a process-global slot. Installing a watcher cancels whatever it replaced, which gives at most one;stop_pet_hover_watcherinstallsNone, which is the stop. The loop selects on the tokenbiasedahead of the tick, so a stop lands before the next round trip instead of one tick after it, and both branches are cancel safe.lib.rsstops it from the pet window's close/destroy branch and again at the top ofExitRequested, before anything there blocks the main thread. A watcher whose window is already gone never gets a close event, which is why the second call is not redundant.What is preserved: while the pet window is open, nothing changes. Same 80 ms interval, same bounds cache, same enter and leave events, on every platform. I considered gating the watcher to macOS, since its doc comment gives a macOS-specific reason for existing, and did not:
PetWindow.tsxhas no DOM hover handler of its own, so these events are the only thing driving the waving animation anywhere, and a gate would silently drop hover-waving on Windows and Linux. I also left the interval alone, since tauri exposes no cursor enter or leave window event to replace the poll with.Tests cover the three invariants of the bookkeeping: a new watcher retires the previous one, a stop cancels the live one, and repeated stops do not leave the next watcher born cancelled.