After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 745517 - gnome-shell crashed with SIGABRT in g_assertion_message()
gnome-shell crashed with SIGABRT in g_assertion_message()
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other Linux
: Normal critical
: 1.8
Assigned To: clutter-maint
clutter-maint
: 753073 756739 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-03-03 11:08 UTC by Cristian Aravena Romero
Modified: 2015-10-17 08:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Avoid crashing when an actor not parented to a toplevel is cloned (1.89 KB, patch)
2015-10-01 17:40 UTC, Owen Taylor
committed Details | Review
_clutter_actor_set_enable_paint_unmapped: don't force an unmap (1009 bytes, patch)
2015-10-01 17:40 UTC, Owen Taylor
committed Details | Review
clutter_actor_update_map_state: Remove a useless warning (1.01 KB, patch)
2015-10-01 17:41 UTC, Owen Taylor
committed Details | Review

Description Cristian Aravena Romero 2015-03-03 11:08:32 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:
  • #0 __GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 56
  • #1 __GI_abort
    at abort.c line 89
  • #2 g_assertion_message
  • #3 g_assertion_message_expr
  • #4 clutter_actor_update_map_state
    at ./clutter-actor.c line 1453
  • #5 clutter_clone_paint
    at ./clutter-clone.c line 182
  • #6 _g_closure_invoke_va
    at /build/buildd/glib2.0-2.43.90/./gobject/gclosure.c line 831
  • #7 g_signal_emit_valist
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3214
  • #8 g_signal_emit
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3361
  • #9 clutter_actor_continue_paint
    at ./clutter-actor.c line 3955
  • #10 clutter_actor_paint
    at ./clutter-actor.c line 3875
  • #11 clutter_actor_real_paint
    at ./clutter-actor.c line 3566
  • #12 _g_closure_invoke_va
    at /build/buildd/glib2.0-2.43.90/./gobject/gclosure.c line 831
  • #13 g_signal_emit_valist
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3214
  • #14 g_signal_emit
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3361
  • #15 clutter_actor_continue_paint
    at ./clutter-actor.c line 3955
  • #16 clutter_actor_paint
    at ./clutter-actor.c line 3875
  • #17 clutter_actor_real_paint
    at ./clutter-actor.c line 3566
  • #18 _g_closure_invoke_va
    at /build/buildd/glib2.0-2.43.90/./gobject/gclosure.c line 831
  • #19 g_signal_emit_valist
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3214
  • #20 g_signal_emit
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3361
  • #21 clutter_actor_continue_paint
    at ./clutter-actor.c line 3955
  • #22 clutter_actor_paint
    at ./clutter-actor.c line 3875
  • #23 shell_generic_container_paint
    at shell-generic-container.c line 139
  • #24 _g_closure_invoke_va
    at /build/buildd/glib2.0-2.43.90/./gobject/gclosure.c line 831
  • #25 g_signal_emit_valist
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3214
  • #26 g_signal_emit
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3361
  • #27 clutter_actor_continue_paint
    at ./clutter-actor.c line 3955
  • #28 clutter_actor_paint
    at ./clutter-actor.c line 3875
  • #29 clutter_stage_paint
    at ./clutter-stage.c line 705
  • #30 meta_stage_paint
    at backends/meta-stage.c line 124
  • #31 _g_closure_invoke_va
    at /build/buildd/glib2.0-2.43.90/./gobject/gclosure.c line 831
  • #32 g_signal_emit_valist
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3214
  • #33 g_signal_emit
    at /build/buildd/glib2.0-2.43.90/./gobject/gsignal.c line 3361
  • #34 clutter_actor_continue_paint
    at ./clutter-actor.c line 3955
  • #35 clutter_actor_paint
    at ./clutter-actor.c line 3875
  • #36 _clutter_stage_do_paint
    at ./clutter-stage.c line 688
  • #37 clutter_stage_cogl_redraw
    at ./cogl/clutter-stage-cogl.c line 566
  • #38 clutter_stage_do_redraw
    at ./clutter-stage.c line 1201
  • #39 _clutter_stage_do_update
    at ./clutter-stage.c line 1259
  • #40 master_clock_update_stages
    at ./clutter-master-clock.c line 463
  • #41 clutter_clock_dispatch
    at ./clutter-master-clock.c line 595
  • #42 g_main_dispatch
    at /build/buildd/glib2.0-2.43.90/./glib/gmain.c line 3122
  • #43 g_main_context_dispatch
    at /build/buildd/glib2.0-2.43.90/./glib/gmain.c line 3737
  • #44 g_main_context_iterate
    at /build/buildd/glib2.0-2.43.90/./glib/gmain.c line 3808
  • #45 g_main_loop_run
    at /build/buildd/glib2.0-2.43.90/./glib/gmain.c line 4002
  • #46 meta_run
    at core/main.c line 473
  • #47 main
    at main.c line 463

Comment 1 Jasper St. Pierre (not reading bugmail) 2015-03-03 16:47:18 UTC
Clutter is complaining about an inconsistent map state. Weird.

Emmanuele, any ideas?
Comment 2 Emmanuele Bassi (:ebassi) 2015-03-03 16:54:11 UTC
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).
Comment 3 Marius Gedminas 2015-04-13 15:38:23 UTC
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.
Comment 4 Jasper St. Pierre (not reading bugmail) 2015-04-13 16:01:12 UTC
Is it possible to somewhat-consistently reproduce it?
Comment 5 Marius Gedminas 2015-04-14 05:02:53 UTC
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>.)
Comment 6 Richard.M.Crowley 2015-05-15 05:03:30 UTC
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.
Comment 7 dirk.seynol 2015-07-16 08:55:27 UTC
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
Comment 8 dirk.seynol 2015-07-16 09:38:47 UTC
Feedback:

from tweak-tool > Appearance > icons

switching the selected theme mostly always push a crash
Comment 9 Emmanuele Bassi (:ebassi) 2015-07-16 10:03:09 UTC
(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:

  • #3 g_assertion_message_expr
    at /build/buildd/glib2.0-2.41.1/./glib/gtestutils.c line 2306
  • #4 meta_ui_get_default_window_icon
    at ui/ui.c line 854

Please, open a new bug.
Comment 10 Owen Taylor 2015-09-29 16:47:05 UTC
*** Bug 753073 has been marked as a duplicate of this bug. ***
Comment 11 Owen Taylor 2015-09-29 17:13:17 UTC
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.
Comment 12 Owen Taylor 2015-10-01 17:40:52 UTC
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.
Comment 13 Owen Taylor 2015-10-01 17:40:59 UTC
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.
Comment 14 Owen Taylor 2015-10-01 17:41:06 UTC
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).
Comment 15 Emmanuele Bassi (:ebassi) 2015-10-01 18:41:12 UTC
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.
Comment 16 Emmanuele Bassi (:ebassi) 2015-10-01 18:41:46 UTC
Review of attachment 312519 [details] [review]:

Sure.
Comment 17 Emmanuele Bassi (:ebassi) 2015-10-01 18:42:13 UTC
Review of attachment 312520 [details] [review]:

Okay.
Comment 18 Owen Taylor 2015-10-01 19:02:23 UTC
(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.
Comment 19 Emmanuele Bassi (:ebassi) 2015-10-01 19:10:02 UTC
(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.
Comment 20 Owen Taylor 2015-10-01 19:53:03 UTC
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
Comment 21 Florian Müllner 2015-10-17 08:03:49 UTC
*** Bug 756739 has been marked as a duplicate of this bug. ***