fix: keep wayland clipboard copies responsive - #3015
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughWayland clipboard writes now wait for startup, then return after ChangesWayland clipboard handling
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Merge Risk: ⚪ Minimal · up to The change prevents Wayland clipboard operations from blocking the application while preserving clipboard delivery and child-process cleanup. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant run_clipboard_command
participant wl-copy
participant clipboard_reaper_thread
run_clipboard_command->>wl-copy: Start process and send clipboard data
wl-copy-->>run_clipboard_command: Become persistent clipboard owner
run_clipboard_command-->>run_clipboard_command: Return success without waiting
clipboard_reaper_thread->>wl-copy: Wait for process exit
clipboard_reaper_thread->>clipboard_reaper_thread: Reap owner and handle exit status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 1 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Yup, |
|
@akbash-bot the implementation review is clean. please rebase onto current master, resolve the changelog conflict, and rerun checks and both review bots. |
|
Rebased onto I cannot update this PR head because |
96d0530 to
334b965
Compare
Greptile SummaryThis PR prevents Wayland clipboard writes from blocking Herdr on a long-lived
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified. The changed Wayland path returns after transferring the long-lived child to a reaper while still detecting immediate startup failures, and the focused tests cover responsiveness, fallback, payload integrity, arguments, and eventual reaping.
|
| Filename | Overview |
|---|---|
| src/platform/linux.rs | Adds bounded startup polling and asynchronous child reaping for wl-copy, preserving synchronous status handling for finite clipboard helpers and adding focused lifecycle tests. |
| docs/next/CHANGELOG.md | Adds an accurate release-note entry for the Wayland clipboard responsiveness fix. |
Sequence Diagram
sequenceDiagram
participant H as Herdr
participant W as wl-copy
participant R as Reaper thread
participant C as Wayland compositor
H->>W: Start with piped clipboard text
H->>W: Poll exit status for up to 100 ms
alt Immediate exit
W-->>H: Success or failure status
else Still running
H->>R: Transfer child handle
H-->>H: Resume control path
W->>C: Serve clipboard selection
C-->>W: Selection replaced
W-->>R: Exit
R->>R: Reap child
end
Reviews (1): Last reviewed commit: "fix: preserve clipboard fallback after w..." | Re-trigger Greptile
Issue
On Wayland, double-clicking a token can freeze Herdr while
wl-copykeeps serving the clipboard selection. Existing and newly attached clients then stop responding.Problem
Herdr waited for
wl-copyto exit after sending the copied text. A clipboard owner can stay alive until another application replaces the selection, so that wait blocked Herdr's control path.How did we fix it?
Herdr now returns after handing the text to
wl-copyand reaps the clipboard owner on a dedicated thread when it eventually exits. Other Linux clipboard helpers still wait for and report their exit status as before.Verification
The regression test blocked before the fix while a fake
wl-copyremained alive. It now verifies immediate return, exact arguments and text, and eventual child reaping. Focused tests, formatting, and clippy pass. The full suite passed 3,487 of 3,488 tests;live_server_holds_one_pty_master_fd_per_panestill fails because its replacement server does not appear, and the same failure reproduces with this change removed.refs #3014