Fix whole-app crash when dragging a tab out of a window; add explicit Detach Tab action#1101
Open
henricook wants to merge 3 commits into
Open
Fix whole-app crash when dragging a tab out of a window; add explicit Detach Tab action#1101henricook wants to merge 3 commits into
henricook wants to merge 3 commits into
Conversation
Dragging a tab out of its window could crash the whole process, killing every window at once (issue gnome-terminator#1022). The gdb log on that issue shows a use-after-free in gtk_notebook_drag_end (gtknotebook.c:3743): our create-window handler removed the page while GTK was still mid-way through its drag bookkeeping, and when the source window had exactly two tabs, hoover() dissolved the notebook entirely, freeing the drag state GTK touches right after the handler returns. That also explains the intermittency: three or more tabs means no notebook collapse. Defer the detach with GObject.idle_add until the drag has fully ended, and queue a re-allocate after failed tab drags so the restored tab label is not left undrawn under Wayland (GTK issue #3143). GTK on Wayland only emits create-window for drops outside any drop-accepting surface (GTK issue gnome-terminator#46), so dragging a tab onto another window can never detach there. Add an explicit, DnD-free way to detach as well: a detach_tab keybinding (Shift+Ctrl+D) and a Detach Tab context menu item, plumbed the same way as move-tab. Tabs containing splits detach with all their panes. The keybinding prefs tests hardcode row indices of the sorted keybinding list; the new binding sorts before edit_* and zoom_in, so those indices shift by one.
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
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.
Background
Occasionally I drag a tab out of my terminator window on Gnome/Wayland and drop it, and it gets lost forever (but I think is still running!).
What
Shift+Ctrl+Dkeybinding + right-click menu item (tabs containing splits detach whole)Why
The gdb log attached to #1022 shows the segfault inside GTK's own drag-end handling:
GTK emits
create-windowin the middle of its drag bookkeeping, and our handler removed the page on the spot. With exactly two tabs,hoover()then dissolved the whole notebook, freeingpriv->dnd_windowbefore GTK'sdrag-endhandler dereferences it (hence intermittent: 3+ tabs, no collapse, no crash). One Python process hosts every window, so "all instances crash".The explicit action matters because on Wayland GTK only emits
create-windowfor drops on shell chrome or bare desktop (GTK#46) - dropping a tab on any window can never detach, and that's not fixable app-side.How
create_window_detachnow just schedules the detach withGObject.idle_add; the page is moved after the drag has fully ended, never inside the emissiondetach-tabterminal signal wired likemove-tab, driving the keybinding and menu itemqueue_resizeafter a failed tab drag so the restored label repaints (GTK#3143)test_prefseditor_keybindings.py(the new binding sorts beforeedit_*)Credit where due: Claude did the gdb spelunking and the nested-compositor testing.