GNOME Bugzilla – Bug 745517
gnome-shell crashed with SIGABRT in g_assertion_message()
Last modified: 2015-10-17 08:03:49 UTC
Open bug in launchpad.net https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1424199 "Shell crash after exiting a WINE application (Battlefield 1942, running Wine 1.7.37)" Backtrace:
+ Trace 234767
Clutter is complaining about an inconsistent map state. Weird. Emmanuele, any ideas?
Clones seem to be involved, according to the stack trace. Other than that, no idea. I run applications under Wine and cannot reproduce (and I don't have Battlefield 1942).
This assertion just fired for me. No Wine involved, I was typing (accidentally) into gnome-shell overview's search box. More specifically, I typed <Super>, "off", <Tab>, <Enter> (intending to launch offlineimap in a terminal, only I missed the 't' in <Super>+t) and gnome-shell crashed. This is on Ubuntu GNOME 14.10, with gnome-shell 3.14 from the ubuntu-gnome PPA. I'm not the only Ubuntu GNOME user who's seen this: https://bugs.launchpad.net/ubuntu-gnome/+bug/1436829 I've a stack trace with local variables etc at https://gist.github.com/mgedmin/34c3d26a21580a8329db. It's an Apport crash dump, with the huge base64-encoded core dump removed to make it not-so-huge. Unfortunately it's missing debug symbols for gnome-shell and libmutter.
Is it possible to somewhat-consistently reproduce it?
I haven't been able to reproduce it. :( (I can gdb the core file with full debug symbols, if you think inspecting local variables can shed any light on this. Assuming their values won't be <optimized out>.)
I can reproduce it fairly regularly, but the trigger seems somewhat random... It causes a freeze for anywhere between 30 seconds and a few minutes, but it only happens once per boot. It usually happens early on (within the first hour). it does frequently occur when searching by pressing super and typing a few letters, but this is not a requirement to cause it. It has nothing to do with WINE as far as I can tell -- I don't even have WINE installed on the machine that gives me this error. It happened consistently on Ubuntu 14.10, and it also happens on Ubuntu 15.4 running gnome 3.14.1. If you need any debug information, just let me know the commands to run/the logs you need and I can get them.
i'm getting lot of such crashes on ubuntu wily i386 with a gnome-shell session. I have installed some 'extensions' like 'taskbar', 'applications', etc and they often are desactivated when a crash happen. So it seems related to a faulty theme (maybe adwaita or else) or some 'icon' needing an upgrade/fix; like these fixed: https://bugs.launchpad.net/ubuntu/+source/marco/+bug/1351942 https://bugs.launchpad.net/noise/+bug/1431042
Feedback: from tweak-tool > Appearance > icons switching the selected theme mostly always push a crash
(In reply to dirk.seynol from comment #7) > i'm getting lot of such crashes on ubuntu wily i386 with a gnome-shell > session. > > I have installed some 'extensions' like 'taskbar', 'applications', etc and > they often are desactivated when a crash happen. > > So it seems related to a faulty theme (maybe adwaita or else) or some > 'icon' needing an upgrade/fix; like these fixed: > > https://bugs.launchpad.net/ubuntu/+source/marco/+bug/1351942 > https://bugs.launchpad.net/noise/+bug/1431042 The stack trace you linked has nothing to do with the stack trace of this bug. Your crash is an assertion failure in Mutter:
+ Trace 235274
Please, open a new bug.
*** Bug 753073 has been marked as a duplicate of this bug. ***
Some reading of the code reveals what the immediate trigger of this crash is: If the source of a clone is actor X, and actor X has a parent, but *that* parent doesn't have a parent (or some ancestor is orphaned several levels up), then that will cause a crash with this assertion. If the clone source itself is orphaned then you get the non-fatal warning: Attempting to map a child that does not meet the necessary invariants: the actor '<X>' has no parent Clutter definitely needs to be fixed to consolidate these cases and produce a clean warning, or just quietly do nothing - I'll take a look at that. It might also be possible to find some cases in GNOME Shell and fix them individually, but I'm not sure that all the reported crashes are even the same thing - there is a lot of usage of Clutter.Clone in GNOME Shell.
Created attachment 312518 [details] [review] Avoid crashing when an actor not parented to a toplevel is cloned If we can't realize the source actor for a clone, simply skip updating the map state and painting it.
Created attachment 312519 [details] [review] _clutter_actor_set_enable_paint_unmapped: don't force an unmap When enable_paint_unmapped is disabled, we shouldn't force the source widget to be unmapped if the constraints would keep it mapped; in practice this shouldn't matter unless a paint handler is messing with the map state.
Created attachment 312520 [details] [review] clutter_actor_update_map_state: Remove a useless warning A check for priv->parent == NULL was inside the else of a check for (priv->parent == NULL).
Review of attachment 312518 [details] [review]: So, a bit of context: one of the reasons why we never checked if a cloned actor was realized was because we allowed people to add an actor somewhere, and only use clones to draw it. I'm talking about pre-1.0 stuff. We still had some code at the time that assumed that behaviour, so we never got around fixing it when we landed the state invariants. The whole Clone actor also suffers from a definite lack of behavioural constraints as to how and what it will render, which is one of the reasons why people end up using it and then complaining when it breaks. Apart from a couple of issues, I think this patch is a good thing to have. ::: clutter/clutter-actor.c @@ +15942,3 @@ + * realized or painted. + */ + if (CLUTTER_ACTOR_IS_REALIZED (self)) Instead of doing a "is-realized" check right after calling realize(), I'd probably bail out earlier if clutter_actor_get_stage() returns NULL, and not even call realize() in the first place. ::: clutter/clutter-clone.c @@ +188,3 @@ + * realized or painted. + */ + if (clutter_actor_is_realized (priv->clone_source)) I'd move this check earlier in the process, right after we check if clone_source is NULL. Otherwise we're just doing a lot of busy work for nothing.
Review of attachment 312519 [details] [review]: Sure.
Review of attachment 312520 [details] [review]: Okay.
(In reply to Emmanuele Bassi (:ebassi) from comment #15) > Review of attachment 312518 [details] [review] [review]: > > So, a bit of context: one of the reasons why we never checked if a cloned > actor was realized was because we allowed people to add an actor somewhere, > and only use clones to draw it. I'm talking about pre-1.0 stuff. We still > had some code at the time that assumed that behaviour, so we never got > around fixing it when we landed the state invariants. > > The whole Clone actor also suffers from a definite lack of behavioural > constraints as to how and what it will render, which is one of the reasons > why people end up using it and then complaining when it breaks. > > Apart from a couple of issues, I think this patch is a good thing to have. > > ::: clutter/clutter-actor.c > @@ +15942,3 @@ > + * realized or painted. > + */ > + if (CLUTTER_ACTOR_IS_REALIZED (self)) > > Instead of doing a "is-realized" check right after calling realize(), I'd > probably bail out earlier if clutter_actor_get_stage() returns NULL, and not > even call realize() in the first place. Hmm, that would introduce a walk up the tree to the toplevel every time a clone is painted, something I don't much like doing on principle. This is there's just a walk up to the toplevel when painting a clone of not-ultimately-parented actor, but in the normal case we can realize the actor once and just use the realized flag. (I introduced the "anchored" flag in GTK+ to make this information local to a widget and to avoid continually walking up the tree.) > ::: clutter/clutter-clone.c > @@ +188,3 @@ > + * realized or painted. > + */ > + if (clutter_actor_is_realized (priv->clone_source)) > > I'd move this check earlier in the process, right after we check if > clone_source is NULL. Otherwise we're just doing a lot of busy work for > nothing. The realization doesn't happen until enable_paint_unmapped is set, so you can't check realization earlier. It *would* be possible to add a clutter_actor_get_stage(source) != null check earlier, but that means *another* walk up the tree each time painting an actor. [Or having only one and leaving calling _clutter_actor_set_enable_paint_unmapped() on an not-ultimately-parented widget crashing with an assertion failure.] Let me know how you'd prefer to do this - I agree that checking realization after trying to realize is a bit depending on side-effects, but it does have efficiency benefits.
(In reply to Owen Taylor from comment #18) > (I introduced the "anchored" flag in GTK+ to make this information local to > a widget and to avoid continually walking up the tree.) I've been meaning to cache the stage returned by clutter_actor_get_stage_internal() for a while; I could have a go at it. > Let me know how you'd prefer to do this - I agree that checking realization > after trying to realize is a bit depending on side-effects, but it does have > efficiency benefits. Since this is a crasher, I think we can go in with your fix, and then do a further optimization pass. So I retract my comments.
OK, pushing as-is for now. Thanks for the reviews! Attachment 312518 [details] pushed as 76c8cd2 - Avoid crashing when an actor not parented to a toplevel is cloned Attachment 312519 [details] pushed as 4c9443b - _clutter_actor_set_enable_paint_unmapped: don't force an unmap Attachment 312520 [details] pushed as 2ddec57 - clutter_actor_update_map_state: Remove a useless warning
*** Bug 756739 has been marked as a duplicate of this bug. ***