GNOME Bugzilla – Bug 791006
Changing resolution/scale might lead to a crash when deferencing a destroyed compositor's top_window_actor
Last modified: 2017-12-01 03:55:10 UTC
This happens repeatedly here when using the resource-scale branch and changing scale and resolution from u-c-c The trace of the crash is:
+ Trace 238205
Look that this might be related to Bug 788493 too.
Ouch, bugzilla ate the rest of my comment after the trace... Let me post it again. However, how is quite clear the issue is because the compositor's top_window_actor doesn't get nullified when that is destroyed. I've tried to track down when such destruction happens but it seems to be driven mostly by the JS engine and its garbage collector... So not too easy to find where this unallocation happens (gjs_dumpstack does say nothing of course). See the trace:
+ Trace 238206
Created attachment 364642 [details] [review] compositor: add a weak-ref to nullify top_window_actor when it's removed Adding a patch to fix this using a weak ref, as said I've not been able to track this way further down, but I think this is still right to do.
Created attachment 364697 [details] [review] compositor: also remove actors from the windows list So, after digging a little bit into this, I found the real cause for this to happen, check this log: Nov 30 13:40:58 ubuntu org.gnome.Shell.desktop[42385]: meta_compositor_sync_stack, top actor set to 0x5623cb0c27f0, meta window is 0x5623cbc94040 Nov 30 13:40:58 ubuntu org.gnome.Shell.desktop[42385]: meta_compositor_sync_stack, top actor set to 0x5623cb0c2bd0, meta window is 0x5623cbc94360 Nov 30 13:41:04 ubuntu org.gnome.Shell.desktop[42385]: meta_compositor_remove_window, Removing window 0x5623cbc94360, actor 0x5623cb0c2bd0 [top is 0x5623cb0c2bd0] Nov 30 13:41:04 ubuntu org.gnome.Shell.desktop[42385]: meta_compositor_remove_window, Top actor was 0x5623cb0c2bd0 and got nullified Nov 30 13:41:04 ubuntu org.gnome.Shell.desktop[42385]: meta_compositor_sync_stack, top actor set to 0x5623cb0c2bd0, meta window is 0x5623cbc94360 Nov 30 13:41:04 ubuntu gnome-shell[42385]: Object Meta.WindowActor (0x5623cb0c2bd0), has been already finalized. Impossible to get any property from it. Nov 30 13:41:04 ubuntu gnome-shell[42385]: Object Meta.WindowActor (0x5623cb0c2bd0), has been already finalized. Impossible to set any property to it. As you can see the top actor is set to 0x5623cb0c2bd0, then this is removed and requested to be destroyed, but at next sync_stack cycle, it's set back again as top_actor. This happens because the actor isn't removed from the compositor->windows list, and thus picked from the old_stack. Updated the patch to also ensure we clear this actor from the said list, and the crash is fixed.
Created attachment 364711 [details] [review] compositor: reset top_window_actor and remove it from windows when destroyed As per discussion con Jonas, we decided it's better to remove the top actor references when it gets really destroyed, as animations might be still happening on the window actor.
Created attachment 364712 [details] [review] window-actor: rename destroy function in maybe_destroy Since this might delay the destruction after animation has succeeded, it's just better to rename this accordingly.
(In reply to Marco Trevisan (Treviño) from comment #4) > Created attachment 364711 [details] [review] [review] > compositor: reset top_window_actor and remove it from windows when destroyed > > As per discussion *con* Jonas Sorry, I'm speaking spanish way too much these days :-D s/con/with/
Review of attachment 364711 [details] [review]: just a naming nit. with that fixed, lgtm ::: src/compositor/compositor.c @@ +954,3 @@ +static void +on_top_stack_actor_destroyed (MetaWindowActor *window_actor, nit: s/top_stack_actor/top_window_actor/
Review of attachment 364712 [details] [review]: Hmm. So "maybe_destroy" to me reads as it should be called multiple times until the maybe became definite. It's more of a "destroy now or later", but right now I can't think of a better name than just "destroy".. (unless its really _destroy_now_or_later() :P)
Patches pushed as commit b1587f0 and commit 1a1db9e