From 7bb9d48dd1383b688c8fefbbb08c552c3cc33c18 Mon Sep 17 00:00:00 2001 From: Tudor Brindus Date: Sun, 11 Oct 2020 21:25:26 -0400 Subject: [PATCH] xwayland: remove stale transfers from the same requestor It seems that if we ever try to reply to a selection request after another has been sent by the same requestor (we reply in FIFO order), the requestor never reads from it, and we end up stalling forever on a transfer that will never complete. It appears that `XCB_SELECTION_REQUEST` has some sort of singleton semantics, and new requests for the same selection are meant to replace outstanding older ones. I couldn't find a reference for this, but empirically this does seem to be the case. Real (contrived) case where we don't currently do this, and things break: * run fcitx * run Slack * wl-copy < <(base64 /opt/firefox/libxul.so) # or some other large file * focus Slack (no need to paste) fcitx will send in an `XCB_SELECTION_REQUEST`, and we'll start processing it. Immediately after, Slack sends its own. fcitx hangs for a long, long time. In the meantime, Slack retries and sends another selection request. We now have two pending requests from Slack. Eventually fcitx gives up (or it can be `pkill`'d), and we start processing the first request Slack gave us (FIFO). Slack (Electron?) isn't listening on the other end anymore, and this transfer never completes. The X11 clipboard becomes unusable until Slack is killed. After this patch, the clipboard is immediately usable again after fcitx bails. Also added a bunch of debug-level logging that makes diagnosing this sort of issue easier. Refs swaywm/sway#4007. --- xwayland/selection/outgoing.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/xwayland/selection/outgoing.c b/xwayland/selection/outgoing.c index 943e4d1a..190fb741 100644 --- a/xwayland/selection/outgoing.c +++ b/xwayland/selection/outgoing.c @@ -71,9 +71,11 @@ static void xwm_selection_transfer_destroy_outgoing( struct wlr_xwm_selection *selection = transfer->selection; bool was_first = transfer == xwm_selection_transfer_get_first(selection); wl_list_remove(&transfer->outgoing_link); + wlr_log(WLR_DEBUG, "Destroying transfer %p", transfer); // Start next queued transfer if we just removed the active one. if (was_first && !wl_list_empty(&selection->outgoing)) { + wlr_log(WLR_DEBUG, "Destroyed transfer was active, starting next"); xwm_selection_transfer_start_outgoing( xwm_selection_transfer_get_first(selection)); } @@ -230,6 +232,8 @@ static void xwm_selection_transfer_start_outgoing( struct wlr_xwm *xwm = transfer->selection->xwm; struct wl_event_loop *loop = wl_display_get_event_loop(xwm->xwayland->wl_display); + wlr_log(WLR_DEBUG, "Starting transfer %p", transfer); + assert(transfer == xwm_selection_transfer_get_first(transfer->selection)); transfer->source = wl_event_loop_add_fd(loop, transfer->source_fd, WL_EVENT_READABLE, xwm_data_source_read, transfer); } @@ -312,14 +316,34 @@ static void xwm_selection_send_data(struct wlr_xwm_selection *selection, transfer->source_fd = p[0]; wlr_log(WLR_DEBUG, "Sending Wayland selection %u to Xwayland window with " - "MIME type %s, target %u", req->target, mime_type, req->target); + "MIME type %s, target %u, transfer %p", req->target, mime_type, + req->target, transfer); xwm_selection_source_send(selection, mime_type, p[1]); + // It seems that if we ever try to reply to a selection request after + // another has been sent by the same requestor, the requestor never reads + // from it. It appears to only ever read from the latest, so purge stale + // transfers to prevent clipboard hangs. + struct wlr_xwm_selection_transfer *outgoing, *tmp; + wl_list_for_each_safe(outgoing, tmp, &selection->outgoing, outgoing_link) { + if (transfer->request.requestor == outgoing->request.requestor) { + wlr_log(WLR_DEBUG, "Destroying stale transfer %p", outgoing); + xwm_selection_send_notify(selection->xwm, &outgoing->request, false); + xwm_selection_transfer_destroy_outgoing(outgoing); + } + } + wl_list_insert(&selection->outgoing, &transfer->outgoing_link); // We can only handle one transfer at a time if (wl_list_length(&selection->outgoing) == 1) { + wlr_log(WLR_DEBUG, "No transfer active, starting %p now", transfer); xwm_selection_transfer_start_outgoing(transfer); + } else { + struct wlr_xwm_selection_transfer *outgoing; + wl_list_for_each(outgoing, &selection->outgoing, outgoing_link) { + wlr_log(WLR_DEBUG, "Transfer %p still queued", outgoing); + } } }