Originally created by: adityaharishch
Summary
CodeRabbit reviewed PR [#553] (pre-main -> main) and flagged 5 issues, 4 of them on start_capture (PR [#552]'s a11y double-free fix). This addresses the 3 that are real; explains why the 4th isn't touched here; and leaves the 5th (a Jira remedy-selection bug on a different commit, not mine) for that PR's author.
Fixed:
- Unbounded hang risk: the old UI-event recorder thread's teardown isn't actually bounded to a few hundred ms as the original doc comment claimed - a backpressured UI-event channel can leave it stuck in blocking_send well past its own 500ms poll. Joining it inline in start_capture could therefore stall whichever thread called it, including a tokio worker when called from an async command handler (e.g. resume). Fix: the join now happens on the NEW recorder's own OS thread, never on start_capture's caller.
- TOCTOU on concurrent restarts: the AppState lock was previously dropped between taking the old thread handle and storing the new one, so two overlapping start_capture calls (e.g. a schedule auto-resume racing a user-triggered resume) could each spawn a generation and clobber each other's stored handle - leaving the loser's generation never joined by a future restart, reopening the exact double-free race PR [#552] exists to close. Fix: the whole stop/spawn/store sequence now runs under one AppState lock held for the entire function.
- Swallowed panic: a panicking previous-generation thread was silently discarded (let _ = handle.join()). Now logged with the panic payload.
Not touched (assessed, not a real fix here): the poisoned-mutex-unwrap suggestion on the two new .lock().unwrap() call sites. This file already uses .lock().unwrap() pervasively - a dozen+ sites in this same function alone - so special-casing only these two would be inconsistent with the rest of the file, not an actual improvement.
Test plan
- [x] cargo check / clippy -D warnings (capture feature) on tray/src-tauri - clean
- [x] cargo test (capture feature) on tray/src-tauri - 151 passed
- [x] Full pre-push suite (fmt + clippy + UI build/tests + security audit + cargo test) - all green
Originally posted by: coderabbitai[bot]
✨ Finishing Touches
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests - [ ] Commit unit tests in branch `fix/pr553-coderabbit-capture-restart`Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)Comment
@coderabbitai helpto get the list of available commands.Ticket changed by: adityaharishch