From 877be654470f8a1a0d8f617f820c36277e38cdc4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 02:39:19 +0000 Subject: [PATCH] Fix crashes when moving tab between OS windows (issue #9677) Wayland (glfw/wl_window.c): - Fix out-of-bounds access in send_drag_data: look up item by MIME type instead of using the data-request index i to index _glfw.drag.items[]. The compositor calls drag_source_send once per target window entered, so _glfw.wl.drag.count grows independently of item_count, causing _glfw.drag.items[i] to be out-of-bounds on the second drag, yielding a garbage optional_data pointer that made write() fail with EFAULT. - Fix protocol error "Drag has not ended": change on_fail and the GLFW_DRAG_DATA_REQUEST error path to call finish_drag_write(i)+return instead of cancel_drag(), which was calling wl_data_source_destroy() before the compositor ended the drag, violating the Wayland protocol. - Fix double-free of dr.pending_data: null the pointer after free and add cleanup to finish_drag_write(). - Fix missing finish_drag_write() after a full write in data-request mode, which left the pipe open causing the target to wait for EOF. X11 (glfw/x11_window.c): - Wrap XSendEvent() calls in send_xdnd_enter/position/leave/drop with _glfwGrabErrorHandlerX11()/_glfwReleaseErrorHandlerX11(). A target window destroyed between discovery and message delivery produced a BadWindow error that hit the default X11 abort handler. Now handled gracefully by clearing current_target or cancelling the drag." Fixes #9677 Fixes #9683 --- glfw/wl_window.c | 52 +++++++++++++++++++++++++++++++---------------- glfw/x11_window.c | 22 +++++++++++++++++++- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/glfw/wl_window.c b/glfw/wl_window.c index 398d96ec0..ff3b8b69c 100644 --- a/glfw/wl_window.c +++ b/glfw/wl_window.c @@ -3177,6 +3177,7 @@ finish_drag_write(size_t i) { dr.watch_id = 0; if (dr.fd > -1) safe_close(dr.fd); dr.fd = -1; + free(dr.pending_data); dr.pending_data = NULL; dr.sz = 0; dr.offset = 0; free((void*)dr.mime_type); dr.mime_type = NULL; } @@ -3198,31 +3199,49 @@ write_as_much_as_possible(int fd, const char *data, size_t sz) { static void send_drag_data(_GLFWwindow *window, size_t i) { ssize_t ret; - bool has_preset_data = _glfw.drag.items[i].data_size > 0; + // Find the item matching the mime type for this data request. + // We cannot use i directly to index _glfw.drag.items because the compositor + // may call drag_source_send multiple times (once per target entered), making + // data_requests grow independently of the items array. + size_t item_idx = _glfw.drag.item_count; // sentinel: not found + for (size_t j = 0; j < _glfw.drag.item_count; j++) { + if (dr.mime_type && _glfw.drag.items[j].mime_type && + strcmp(_glfw.drag.items[j].mime_type, dr.mime_type) == 0) { + item_idx = j; break; + } + } + if (item_idx == _glfw.drag.item_count) { + _glfwInputError(GLFW_PLATFORM_ERROR, + "Wayland: compositor requested data for unrecognised MIME type: %s", + dr.mime_type ? dr.mime_type : "(null)"); + } + bool has_preset_data = item_idx < _glfw.drag.item_count && _glfw.drag.items[item_idx].data_size > 0; + // On write error, only close this pipe; do NOT destroy the wl_data_source + // since the compositor must send cancelled/dnd_finished before that is safe. #define on_fail _glfwInputError(\ GLFW_PLATFORM_ERROR, "Wayland: failed to write drag source data to pipe with error: %s", strerror(errno)); \ - cancel_drag(GLFW_DRAG_CANCELLED); + finish_drag_write(i); return; if (dr.sz > dr.offset) { ret = write_as_much_as_possible(dr.fd, dr.pending_data + dr.offset, dr.sz - dr.offset); if (ret < 0) { on_fail; } else { dr.offset += ret; if (dr.offset >= dr.sz) { - free(dr.pending_data); dr.sz = 0; dr.offset = 0; - if (has_preset_data) finish_drag_write(i); + free(dr.pending_data); dr.pending_data = NULL; dr.sz = 0; dr.offset = 0; + finish_drag_write(i); } } } else if (has_preset_data) { - do { ret = write(dr.fd, _glfw.drag.items[i].optional_data, _glfw.drag.items[i].data_size); } while (ret < 0 && errno == EINTR); + do { ret = write(dr.fd, _glfw.drag.items[item_idx].optional_data, _glfw.drag.items[item_idx].data_size); } while (ret < 0 && errno == EINTR); if (ret < 0) { on_fail; } else { - if ((size_t)ret >= _glfw.drag.items[i].data_size) { + if ((size_t)ret >= _glfw.drag.items[item_idx].data_size) { finish_drag_write(i); } else { - void *pending = malloc(_glfw.drag.items[i].data_size - ret); - if (!pending) { on_fail; } else { - dr.pending_data = pending; dr.sz = _glfw.drag.items[i].data_size - ret; dr.offset = 0; + void *pending = malloc(_glfw.drag.items[item_idx].data_size - ret); + if (!pending) { errno = ENOMEM; on_fail; } else { + dr.pending_data = pending; dr.sz = _glfw.drag.items[item_idx].data_size - ret; dr.offset = 0; } } } @@ -3231,21 +3250,20 @@ send_drag_data(_GLFWwindow *window, size_t i) { _glfwInputDragSourceRequest(window, &ev); if (ev.err_num) { if (ev.err_num == EAGAIN) { removeWatch(&_glfw.wl.eventLoopData, dr.watch_id); dr.watch_id = 0; } - else cancel_drag(GLFW_DRAG_CANCELLED); + else { finish_drag_write(i); return; } } else { if (ev.data_sz) { ret = write_as_much_as_possible(dr.fd, ev.data, ev.data_sz); - if (ret >= 0) { - if ((size_t)ret < ev.data_sz) { - void *pending = malloc(ev.data_sz - ret); - if (!pending) { on_fail; } else { - dr.pending_data = pending; dr.sz = ev.data_sz - ret; dr.offset = 0; - memcpy(pending, ev.data + ret, dr.sz); - } + if (ret >= 0 && (size_t)ret < ev.data_sz) { + void *pending = malloc(ev.data_sz - ret); + if (!pending) { ret = -1; } else { + dr.pending_data = pending; dr.sz = ev.data_sz - ret; dr.offset = 0; + memcpy(pending, ev.data + ret, dr.sz); } } _glfwInputDragSourceRequest(window, &ev); if (ret < 0) { on_fail; } + else if ((size_t)ret >= ev.data_sz) { finish_drag_write(i); } } else finish_drag_write(i); } } diff --git a/glfw/x11_window.c b/glfw/x11_window.c index 7650fb90b..e57aefe66 100644 --- a/glfw/x11_window.c +++ b/glfw/x11_window.c @@ -4048,8 +4048,11 @@ send_xdnd_enter(Window target, int version) { } } + _glfwGrabErrorHandlerX11(); XSendEvent(_glfw.x11.display, target, False, NoEventMask, &event); XFlush(_glfw.x11.display); + _glfwReleaseErrorHandlerX11(); + if (_glfw.x11.errorCode == BadWindow) _glfw.x11.drag.current_target = None; } // Send XdndPosition message to target window @@ -4068,9 +4071,12 @@ send_xdnd_position(Window target, int root_x, int root_y, Time timestamp) { event.xclient.data.l[3] = timestamp; event.xclient.data.l[4] = _glfw.x11.drag.action_atom; + _glfwGrabErrorHandlerX11(); XSendEvent(_glfw.x11.display, target, False, NoEventMask, &event); XFlush(_glfw.x11.display); - _glfw.x11.drag.waiting_for_status = true; + _glfwReleaseErrorHandlerX11(); + if (_glfw.x11.errorCode == BadWindow) _glfw.x11.drag.current_target = None; + else _glfw.x11.drag.waiting_for_status = true; } // Send XdndLeave message to target window @@ -4085,8 +4091,11 @@ send_xdnd_leave(Window target) { event.xclient.format = 32; event.xclient.data.l[0] = _glfw.x11.drag.source_window; + _glfwGrabErrorHandlerX11(); XSendEvent(_glfw.x11.display, target, False, NoEventMask, &event); XFlush(_glfw.x11.display); + _glfwReleaseErrorHandlerX11(); + // BadWindow on leave is benign – the target window is already gone } // Send XdndDrop message to target window @@ -4103,8 +4112,19 @@ send_xdnd_drop(Window target, Time timestamp) { event.xclient.data.l[1] = 0; // Reserved event.xclient.data.l[2] = timestamp; + _glfwGrabErrorHandlerX11(); XSendEvent(_glfw.x11.display, target, False, NoEventMask, &event); XFlush(_glfw.x11.display); + _glfwReleaseErrorHandlerX11(); + if (_glfw.x11.errorCode == BadWindow) { + // Target window was destroyed; cancel the drag gracefully + _GLFWwindow *window = _glfwWindowForId(_glfw.drag.window_id); + if (window) { + GLFWDragEvent ev = {.type = GLFW_DRAG_CANCELLED}; + _glfwInputDragSourceRequest(window, &ev); + } + _glfwFreeDragSourceData(); + } } // Render thumbnail pixels into _glfw.x11.drag.thumbnail_pixmap / thumbnail_gc.