Skip to content

fix(coordinator): add shutdown flag to supervisor + dispatcher loops [中] - #392

Merged
H-Chris233 merged 1 commit into
betafrom
fix/audit-loop-exit-signals
May 9, 2026
Merged

fix(coordinator): add shutdown flag to supervisor + dispatcher loops [中]#392
H-Chris233 merged 1 commit into
betafrom
fix/audit-loop-exit-signals

Conversation

@appergb

@appergbappergb commented May 9, 2026

Copy link
Copy Markdown
Collaborator

User description

Summary

Five hotkey supervisor loops in `coordinator.rs` (dictation / qa / combo / translation / action) and the global-hotkey dispatcher loop in `global_hotkey_runtime.rs` run as `loop { ... sleep N }` with no exit signal.

In production the OS reaps these threads at process exit, so this isn't a user-visible bug today. But:

  • Integration tests can't drive Coordinator up/down without leaking threads across test runs.
  • Future `RunEvent::Exit` cleanup work has nowhere to plug in.
  • A panicked listener thread leaves the supervisor spinning forever with no way to surface the failure.

Change

  • `Inner` gains an `AtomicBool shutdown` field. Each supervisor loop reads it at the top of each iteration (before any work / sleep) and `return`s when set.
  • `GlobalHotkeyRuntime` gains a parallel `AtomicBool shutdown`; the dispatcher loop checks it before each 250 ms `recv_timeout`.
  • Public methods `Coordinator::request_shutdown` and `GlobalHotkeyRuntime::request_shutdown` set the respective flags. `#[allow(dead_code)]` until the `RunEvent::Exit` wiring lands — this PR is infrastructure-only.

Why this is safe

The change is passive: nothing in the production binary sets the flags, so behavior is unchanged. Tests that want bounded teardown can now call `request_shutdown` explicitly. The check is a single `load(SeqCst)` per iteration — overhead is unmeasurable.

Audit linkage

Audit IDs 3.1.2 (loop exit signal, 中) + 3.4.4 (dispatcher loop, 低). Both CONFIRMED. `3.1.3` (JoinHandle storage) is deferred — without a supervisor consumer for `is_finished()` liveness checks, storing the handle is a no-op. See `docs/audit-2026-05-10-validated.md` (local).

Test plan

  • `cargo test --lib` — 183/183 pass.
  • `cargo check --lib` clean.
  • CI build — to be verified.
  • Future: wire `Coordinator::request_shutdown` + `GlobalHotkeyRuntime::request_shutdown` from `RunEvent::Exit` in `lib.rs` so app-quit teardown is bounded. Out of scope for this PR.

PR Type

Enhancement, Tests


Description

  • Add shutdown AtomicBool to Inner struct and supervisor loops

  • Add shutdown AtomicBool to GlobalHotkeyRuntime and dispatcher loop

  • Expose request_shutdown methods for integration tests and future Exit hook


Diagram Walkthrough

flowchart LR
A["Coordinator::request_shutdown()"] --> B["Inner::shutdown"]
B --> C["hotkey supervisor loops (dictation, qa, combo, translation, action)"]
D["GlobalHotkeyRuntime::request_shutdown()"] --> E["GlobalHotkeyRuntime::shutdown"]
E --> F["global hotkey dispatcher loop"]
Loading

File Walkthrough

Relevant files
Enhancement
coordinator.rs
Add shutdown flag to coordinator's hotkey supervisor loops

openless-all/app/src-tauri/src/coordinator.rs

  • Added shutdown: AtomicBool field to Inner struct
  • Initialized shutdown to false in both constructor variants
  • Added public request_shutdown method to set the flag
  • Checked shutdown flag at top of each supervisor loop
  • Marked request_shutdown as #[allow(dead_code)]
+31/-0
global_hotkey_runtime.rs
Add shutdown flag to global hotkey dispatcher loop

openless-all/app/src-tauri/src/global_hotkey_runtime.rs

  • Added shutdown: AtomicBool field to GlobalHotkeyRuntime
  • Imported AtomicBool and Ordering
  • Initialized shutdown to false in shared() constructor
  • Added public request_shutdown method
  • Dispatcher loop checks shutdown before each recv_timeout
+17/-0

… loops
The five hotkey supervisor loops in coordinator.rs (dictation / qa /
combo / translation / action) and the global-hotkey dispatcher loop in
global_hotkey_runtime.rs run as `loop { ... sleep N }` with no exit
signal. In production the OS reaps these threads at process exit, but:
- Integration tests can't drive coordinator up/down without leaking
threads across runs.
- Future RunEvent::Exit cleanup work has nowhere to plug in.
- A panicked listener thread leaves the supervisor spinning forever
with no way to surface the failure.
Fix:
- Inner gains an `AtomicBool shutdown` field; each supervisor loop
reads it at the top of each iteration (before any work / sleep) and
returns when set.
- GlobalHotkeyRuntime gains a parallel AtomicBool shutdown; dispatcher
loop checks it before each 250 ms recv_timeout.
- Public methods Coordinator::request_shutdown and
GlobalHotkeyRuntime::request_shutdown set the respective flags.
Marked #[allow(dead_code)] until the RunEvent::Exit hook lands —
infrastructure-only change for now.
Crucially this is *passive*: nothing sets the flags in the production
binary, so behavior is unchanged. Tests that want bounded teardown can
now call request_shutdown explicitly.
Audit IDs 3.1.2 + 3.4.4 (CONFIRMED 中 + 低). 3.1.3 (JoinHandle storage)
is deferred — without a supervisor consumer for is_finished()
liveness checks, storing the handle is no-op.
Test: 183/183 lib tests pass. cargo check clean.
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@H-Chris233
H-Chris233 merged commit 01c3424 into betaMay 9, 2026
4 checks passed
@appergbappergb mentioned this pull request May 10, 2026
5 tasks
pullBot pushed a commit to yimmy23/openless that referenced this pull request May 10, 2026
10 PRs landed on beta this cycle:
- Open-Less#377 paste shortcut configurable (issue Open-Less#360)
- Open-Less#386 TS UserPreferences updateChannel alignment
- Open-Less#387 focus_target leak on Processing-phase cancel
- Open-Less#388 [严重] MacHotkeyAdapter::shutdown stops CFRunLoop + tap
- Open-Less#389 emit_capsule window.show/hide off audio thread
- Open-Less#390 QA / dictation hotkey routing race
- Open-Less#391 audio-mute spawn_blocking (async hygiene)
- Open-Less#392 hotkey supervisor + global dispatcher exit signal
- Open-Less#393 post-audit logic-review hotfixes (QA mute .await + focus_target Processing branch)
- Open-Less#394 in-process credentials cache (kills repeated Keychain prompts)
Bump 4 files: package.json, tauri.conf.json, Cargo.toml, Cargo.lock.
@appergb
appergb deleted the fix/audit-loop-exit-signals branch May 10, 2026 10:14
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@appergb@H-Chris233