Hey — first off, sorry for the amount of traffic I've been sending this repo lately (a couple of PRs and now this). search-light ships enabled-by-default in a small GNOME distro I put together, so I've got real skin in the game getting it solid on GNOME 50, and I'd much rather get fixes upstream than carry patches downstream forever. Not trying to flood you — happy to consolidate this however suits you best.
On GNOME 50 (Clutter 18) search-light takes the whole shell down with a SIGABRT in a bunch of situations — clicking the panel icon, the Super+Space accelerator, launching an app from the overlay, pressing Escape to close it, the overlay losing focus to a popup, or a window going fullscreen. On Wayland that's a full session kill, and after a couple of times GNOME drops into safe-mode with all extensions off.
I grabbed coredumps for the different triggers and they all come back to the same thing: the extension changes the Clutter actor tree synchronously while an input/gesture/signal is still being dispatched. show() / hide() / _acquire_ui() / _release_ui() reparent the entry (add_child/remove_child), grab key focus, and connect/disconnect global stage signals — and they get called straight from the button-press gesture, the keyboard accelerators, the notify::key-focus / Escape / fullscreen handlers, and the _release_ui teardown.
That was fine on older Clutter, but 18 tightened its invariants and refuses actor surgery mid-dispatch. The aborts I saw:
clutter-gesture.c:544 set_state assertion (new_state == CLUTTER_GESTURE_STATE_POSSIBLE) — panel button-press + keyboard-accelerator paths.clutter-actor.c:1989 clutter_actor_real_unrealize assertion (!clutter_actor_is_mapped), and the set_mapped (!CLUTTER_ACTOR_IN_MAP_UNMAP) assertion — the _release_ui path and a re-entrant hide().
The one change that fixes all of them is to get the actor work out of the dispatch: defer every show()/hide() that originates in an input/gesture/signal handler by one GLib.idle_add tick, so the reparent/focus/signal changes run after the current dispatch unwinds. Per-site point fixes work but you have to chase every entry point.
I already opened #164 (hide the entry before detaching it in _release_ui, plus a re-entrancy guard in hide()) and #165 (defer the panel toggle out of the press gesture). Those are real but only cover the click + teardown paths. The ones still left are the two accel.listenFor() handlers (Super+Space), the Escape-key hide(), the popup-focus-loss hide() in _hidePopups, and _onFullScreen()'s hide() — same idle_add deferral.
This looks like the same root cause behind #82, #133 and #86, and it's in the way of the GNOME 50 work in #155/#156. Happy to roll the whole thing into one PR against whichever GNOME-50 branch you'd prefer, if that's easier to review than the piecemeal ones.
Hey — first off, sorry for the amount of traffic I've been sending this repo lately (a couple of PRs and now this). search-light ships enabled-by-default in a small GNOME distro I put together, so I've got real skin in the game getting it solid on GNOME 50, and I'd much rather get fixes upstream than carry patches downstream forever. Not trying to flood you — happy to consolidate this however suits you best.
On GNOME 50 (Clutter 18) search-light takes the whole shell down with a SIGABRT in a bunch of situations — clicking the panel icon, the Super+Space accelerator, launching an app from the overlay, pressing Escape to close it, the overlay losing focus to a popup, or a window going fullscreen. On Wayland that's a full session kill, and after a couple of times GNOME drops into safe-mode with all extensions off.
I grabbed coredumps for the different triggers and they all come back to the same thing: the extension changes the Clutter actor tree synchronously while an input/gesture/signal is still being dispatched.
show()/hide()/_acquire_ui()/_release_ui()reparent the entry (add_child/remove_child), grab key focus, and connect/disconnect global stage signals — and they get called straight from the button-press gesture, the keyboard accelerators, thenotify::key-focus/ Escape / fullscreen handlers, and the_release_uiteardown.That was fine on older Clutter, but 18 tightened its invariants and refuses actor surgery mid-dispatch. The aborts I saw:
clutter-gesture.c:544set_stateassertion (new_state == CLUTTER_GESTURE_STATE_POSSIBLE) — panel button-press + keyboard-accelerator paths.clutter-actor.c:1989clutter_actor_real_unrealizeassertion (!clutter_actor_is_mapped), and theset_mapped(!CLUTTER_ACTOR_IN_MAP_UNMAP) assertion — the_release_uipath and a re-entranthide().The one change that fixes all of them is to get the actor work out of the dispatch: defer every
show()/hide()that originates in an input/gesture/signal handler by oneGLib.idle_addtick, so the reparent/focus/signal changes run after the current dispatch unwinds. Per-site point fixes work but you have to chase every entry point.I already opened #164 (hide the entry before detaching it in
_release_ui, plus a re-entrancy guard inhide()) and #165 (defer the panel toggle out of the press gesture). Those are real but only cover the click + teardown paths. The ones still left are the twoaccel.listenFor()handlers (Super+Space), the Escape-keyhide(), the popup-focus-losshide()in_hidePopups, and_onFullScreen()'shide()— sameidle_adddeferral.This looks like the same root cause behind #82, #133 and #86, and it's in the way of the GNOME 50 work in #155/#156. Happy to roll the whole thing into one PR against whichever GNOME-50 branch you'd prefer, if that's easier to review than the piecemeal ones.