fix(clipboard): bound channels and coalesce ui notifications#73
Merged
Merged
Conversation
Replace async_channel::unbounded with bounded(256) for both the clipboard event channel (listener -> persistence) and the UI refresh notification channel (persistence -> foreground). Apps that copy several times per second could grow either channel without limit if the consumer momentarily fell behind, and every saved record triggered a full get_display_records + GPUI refresh. Changes: - src/app.rs: introduce CLIPBOARD_EVENT_CHANNEL_CAPACITY and UI_NOTIFY_CHANNEL_CAPACITY (=256). Coalesce notifications by draining all pending () after each recv, so a burst of saves results in a single repository read + UI refresh. Use try_send on the producer side: a full channel means a refresh is already pending and the notification can be dropped (debug-logged). - src/clipboard/listener.rs: switch send_blocking -> try_send in the OS clipboard callback path so a saturated channel never stalls system clipboard notifications. The image-processing background task uses async send().await for natural backpressure. - Add unit tests for drain_pending_notifications and the bounded capacity of both channels. Refs #72
… queue Address review feedback on PR #73: 1. LastCopyState was being advanced even when try_send dropped the event due to a full channel. Once the channel drained, the dedup gate (should_forward_*) would silently filter out the next legitimate retry of the same content, escalating 'drop one event' into 'permanently miss this content'. Fix: advance LastCopyState only after a successful enqueue, in lockstep with the channel. 2. The image processing channel was still async_channel::unbounded despite carrying the heaviest payload (DynamicImage). High-frequency image copies could grow memory without limit, defeating the bounded- channels goal of this PR. Fix: switch to bounded(1) with newest-wins overwrite: when the slot is full, the OS callback drains the oldest queued image and inserts the new one — the most recent image is the only meaningful payload. Implementation: - src/clipboard/listener.rs: - Extract on_clipboard_change body into ClipboardMonitor::dispatch_payload, returning Option<LastCopyState>; the caller advances state only on Some. The mutex guard is now held only across dedup-check + dispatch + state advance (also resolves clippy::significant_drop_tightening). - Add IMAGE_PROCESSING_CHANNEL_CAPACITY = 1 and try_send_image_overwrite helper that uses a producer-side Receiver clone (image_drain) to evict the oldest queued image on TrySendError::Full. - start_clipboard_monitor builds the image channel via bounded(1) and hands a Receiver clone to ClipboardMonitor as the drain handle. - New unit tests cover: - try_send_image_overwrite: empty / full-overwrite / closed paths. - Text + Files branches: state stays unchanged on send failure, so the same content can still be forwarded after the channel drains. - Text branch happy path: state advances on success. Refs #72
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
1 participant
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.
Summary
Replace
async_channel::unboundedwithbounded(256)for the clipboard event and UI notification channels insrc/app.rs, and coalesce UI refresh notifications so a burst of clipboard saves results in a single repository read + GPUI refresh. Also switch the OS clipboard callback path fromsend_blockingtotry_sendso a saturated channel can never stall system clipboard notifications.Linked Issue
Closes #72
Changes
src/app.rs:CLIPBOARD_EVENT_CHANNEL_CAPACITYandUI_NOTIFY_CHANNEL_CAPACITY(both256).async_channel::bounded.try_send; aFullresult is debug-logged as a coalesced refresh,Closedis warned.drain_pending_notificationsafter eachrecv().awaitto coalesce all pending()into a single repository read + UI refresh.src/clipboard/listener.rs:send_blocking→try_sendinClipboardHandler::on_clipboard_changeso the OS clipboard callback thread never blocks on a full channel; dropped events are warn-logged.send().awaitfor natural backpressure (still off the OS callback thread).drain_pending_notifications(drain + empty cases) and the bounded capacity of both channels.Testing
scripts/precheck.shpasses locally (415 tests pass, clippy clean, i18n/icons/themes OK)