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 755954 - Crash when accessing Gtk.Application.add_window()
Crash when accessing Gtk.Application.add_window()
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
3.16.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-10-01 17:49 UTC by Christian Stadelmann
Modified: 2015-10-01 18:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backtrace from the program above run in python 3 under gdb (39.00 KB, text/plain)
2015-10-01 17:49 UTC, Christian Stadelmann
  Details
app: Warn when trying to add windows on an inert instance (1.08 KB, patch)
2015-10-01 18:01 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
app: Warn when trying to add windows on an inert instance (1.89 KB, patch)
2015-10-01 18:22 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review

Description Christian Stadelmann 2015-10-01 17:49:49 UTC
Created attachment 312522 [details]
backtrace from the program above run in python 3 under gdb

This simple python code crashes both python2 and python3:

from gi.repository import Gtk

app = Gtk.Application()
window = Gtk.ApplicationWindow()
app.add_window(window)

Version information (Fedora 22):
python-2.7.10-8.fc22.x86_64
python3-3.4.2-6.fc22.x86_64
glib2-2.44.1-2.fc22.x86_64
gtk3-3.16.7-1.fc22.x86_64
gobject-introspection-1.44.0-1.fc22.x86_64
pygobject2-2.28.6-13.fc22.x86_64
pygobject3-3.16.2-1.fc22.x86_64
python3-gobject-3.16.2-1.fc22.x86_64
Comment 1 Matthias Clasen 2015-10-01 17:56:35 UTC
That is not how GtkApplication is supposed to be used. Adding windows is expected to happen during or after ::startup, not just out of the blue.
Comment 2 Emmanuele Bassi (:ebassi) 2015-10-01 17:57:01 UTC


  • #0 gtk_application_impl_window_added

The implementation is created inside the startup vfunc(), which is called once the application started running.

The proper way to create application windows is to connect/override the activate signal, and add windows there.

We could add an assertion or a critical warning there, but it's not something Python is going to pick up anyway.
Comment 3 Emmanuele Bassi (:ebassi) 2015-10-01 18:01:35 UTC
Created attachment 312523 [details] [review]
app: Warn when trying to add windows on an inert instance

Application windows can only be added after the application has been
started.
Comment 4 Matthias Clasen 2015-10-01 18:05:35 UTC
Review of attachment 312523 [details] [review]:

::: gtk/gtkapplication.c
@@ +1044,3 @@
+                  "GApplication::startup signal has been emitted.");
+      return;
+    }

I think the G_UNLIKELY is unnecessary here... this is not something you're going to do in a tight loop.
But the warning is a nice idea - there's probably more GtkApplication API that should have this.
Comment 5 Emmanuele Bassi (:ebassi) 2015-10-01 18:17:57 UTC
(In reply to Matthias Clasen from comment #4)
> Review of attachment 312523 [details] [review] [review]:
> 
> ::: gtk/gtkapplication.c
> @@ +1044,3 @@
> +                  "GApplication::startup signal has been emitted.");
> +      return;
> +    }
> 
> I think the G_UNLIKELY is unnecessary here... this is not something you're
> going to do in a tight loop.

True. Will fix that before pushing.

I'll also use g_application_get_is_registered(), since that's what we use already.

> But the warning is a nice idea - there's probably more GtkApplication API
> that should have this.

We have various:

  g_return_if_fail (g_application_get_is_registered (…

here and there, but obviously that does not tell you much about why the method failed — and there's the pesky issue of g_return_* macros being optionally disabled.

We should probably use an actual warning instead, like we do inside GApplication to tell you to connect/override activate.
Comment 6 Emmanuele Bassi (:ebassi) 2015-10-01 18:22:21 UTC
Created attachment 312524 [details] [review]
app: Warn when trying to add windows on an inert instance

Application windows can only be added after the application has been
started.
Comment 7 Emmanuele Bassi (:ebassi) 2015-10-01 18:24:43 UTC
Attachment 312524 [details] pushed as 707a071 - app: Warn when trying to add windows on an inert instance