Looping while selecting the same object is causing the application to die raising a dbus no reply exception

Bug #1283966 reported by Dan Chapman  on 2014-02-24
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gedit
Fix Released
Medium
autopilot-gtk (Ubuntu)
High
Unassigned
gedit (Ubuntu)
Undecided
Martin Pitt

Bug Description

This bug seems to have raised it's head again. While trying to reproduce the ubiquity crash I came across this bug.

I'm not sure if it's the cause of bug 1267116 since in that test we are doing the same but we get a crash file which is what I cannot reproduce. So I have filed it as a seperate bug.

It occurs when polling on the same/couple of objects, eventually the app hangs and a dbus blocking no reply exception is raised

I have attached a simple testcase using gedit that reproduces it

Dan Chapman  (dpniel) wrote :
Nicholas Skaggs (nskaggs) wrote :

Confirming the simple testcase Dan left does indeed cause a dbus timeout. I tweaked his testcase a little to see when it fails, looks like @ loop 734 everytime for me at least.

Changed in autopilot-gtk (Ubuntu):
status: New → Confirmed

It renders essential functionality of the package (or a dependent one) broken.

Changed in autopilot-gtk (Ubuntu):
importance: Undecided → High
status: Confirmed → Triaged
Changed in autopilot-gtk:
status: New → Confirmed
Martin Pitt (pitti) wrote :

Thanks Nicholas for the reproducer! I tried some variations to narrow down the scope (as it's not easy to get an actual crash/core dump/backtrace of that):

 * With gnome-calculator it doesn't crash (GeditWindow → MathWindow, GtkToolButton/name='EditUndo' → GtkButton/BuilderName='calc_undo_button'

 * With gedit it doesn't crash if I call self.app.select_single() instead of self.main_window.select_single(), i. e. the additional self.app.select_single('GeditWindow') in the loop (through the main_window property) is significant.

We have a very similar stress test in the test suite already (from the last attempt at fixing that): http://bazaar.launchpad.net/~autopilot/autopilot-gtk/trunk/view/head:/tests/autopilot/tests/test_properties.py#L140 . Bumping that to 1000 and up doesn't crash, but this uses self.app.

 * I tried to make test_stress() more similar to the test case by looping over

   w = self.app.select_single(BuilderName='window_app').select_single('GtkButton', label='gtk-quit')

but that also does not crash.

Martin Pitt (pitti) wrote :

 * Including the self.app.select_single('GeditWindow') in the loop (as property, or just inline) crashes, while calling it once before the loop ("mw = self.app.select_single('GeditWindow')") does not crash.

 * Merely looping over self.app.select_single('GeditWindow'), and dropping the EditUndo select still crashes. That happens even earlier, at ~ 360 iterations.

 * Each iteration causes

   [GLib-GObject] UNKNOWN: g_value_set_enum: assertion 'G_VALUE_HOLDS_ENUM (value)' failed

   which becomes easier/faster to see with G_DEBUG=fatal-criticals

This should be investigated first, to either see if that's the cause of memory corruption or to get it out of the way.

Martin Pitt (pitti) wrote :

The CRITICAL happens when running

        g_value_init(&value, param_spec->value_type);
        g_object_get_property(object_, g_param_spec_get_name(param_spec), &value);

on the property "state" which has GType GeditWindowState, derived from GFlags. This sounds like a bug in glib itself.

If I filter out the "state" property, then the gedit test case runs happily (all 10.000 iterations). This seems to indicate that some kind of memory corruption happens at this point.

Martin Pitt (pitti) wrote :

Ah, that's a bug in gedit actually. gedit/gedit-window.c declares the "state" property as GFlags:
……………………g_object_class_install_property (object_class,
………………………………………………………………………………………………………… PROP_STATE,
………………………………………………………………………………………………………… g_param_spec_flags ("state",
…………………………………………………………………………………………………………………………………………………… "State",
…………………………………………………………………………………………………………………………………………………… "The window's state",
…………………………………………………………………………………………………………………………………………………… GEDIT_TYPE_WINDOW_STATE,
…………………………………………………………………………………………………………………………………………………… GEDIT_WINDOW_STATE_NORMAL,
…………………………………………………………………………………………………………………………………………………… G_PARAM_READABLE |
…………………………………………………………………………………………………………………………………………………… G_PARAM_STATIC_STRINGS));

But gedit_window_get_property() calls g_value_set_enum() on the GValue; it should be g_value_set_flags().

Changed in gedit (Ubuntu):
status: New → Triaged
affects: autopilot-gtk → gedit
Changed in gedit:
importance: Undecided → Unknown
status: Confirmed → Unknown
Martin Pitt (pitti) wrote :

Et voilà, this already got fixed upstream in January: https://git.gnome.org/browse/gedit/commit/gedit/gedit-window.c?id=816957

Changed in gedit (Ubuntu):
status: Triaged → Fix Committed
Martin Pitt (pitti) wrote :

I committed the upstream fix into the packaging branch, built it locally, and confirm that with that Nick's test case runs flawlessly.

I'll upload the fix after the beta-2 freeze.

Changed in autopilot-gtk (Ubuntu):
status: Triaged → Invalid
Changed in gedit (Ubuntu):
assignee: nobody → Martin Pitt (pitti)
Martin Pitt (pitti) wrote :

So I'm afraid this isn't bug 1267116, and we are back to square one :-(

Changed in gedit:
importance: Unknown → Medium
status: Unknown → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gedit - 3.10.4-0ubuntu4

---------------
gedit (3.10.4-0ubuntu4) trusty; urgency=medium

  * Add 00git_state_property_type.patch: Fix type of GeditWindow state
    property, to avoid crashes with autopilot when introspecting it.
    (LP: #1283966)
 -- Martin Pitt <email address hidden> Wed, 26 Mar 2014 15:34:58 +0100

Changed in gedit (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.