GNOME Bugzilla – Bug 745225
eventcontroller: Don't crash if the widget is destroyed first
Last modified: 2015-03-02 10:57:13 UTC
Crashes on latest continuous 20150226.42. gnome-documents 3.15.90-19-g6ebddf816f321b7e0121bde55de4ab4e58557e42: PID: 1976 (gjs-console) UID: 1000 (test) GID: 1000 (test) Signal: 11 (SEGV) Timestamp: Thu 2015-02-26 15:24:33 GMT (53s ago) Command Line: /usr/bin/gjs-console -I /usr/share/gnome-documents/js -c const Main = imports.main; Main.start(); --gapplication-service Executable: /usr/bin/gjs-console Control Group: /user.slice/user-1000.slice/session-6.scope Unit: session-6.scope Slice: user-1000.slice Session: 6 Owner UID: 1000 (test) Boot ID: 6d9c3de245234b36b33251627810a1bc Machine ID: 45bb3b96146aa94f299b9eb43646eb35 Hostname: qemux86-64 Coredump: /var/lib/systemd/coredump/core.gjs-console.1000.6d9c3de245234b36b33251627810a1bc.1976.1424964273000000.xz Message: Process 1976 (gjs-console) of user 1000 dumped core. Core was generated by `/usr/bin/gjs-console -I /usr/share/gnome-documents/js -c const Main = imports.m'. Program terminated with signal SIGSEGV, Segmentation fault.
+ Trace 234721
This is what happens: gesture = gtk_gesture_multi_press_new (widget); gtk_widget_destroy (widget); g_object_unref (gesture); We could work it around in the application by swapping the order of the last two lines, but I think we should fix gtk+.
Created attachment 298203 [details] [review] eventcontroller: Don't crash if the widget is destroyed first
Here is a better back trace: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff759ff4d in _gtk_widget_remove_controller (widget=0xca2320, controller=0xc7cf90) at gtkwidget.c:17242 17242 g_return_if_fail (GTK_IS_WIDGET (widget)); (gdb) bt
+ Trace 234733
Thread 1 (Thread 0x7ffff7fb5a00 (LWP 30828))
This is now crashing because of commit b8e87d47626afd7d6271425a5772f2ee774d1b78 Author: Carlos Garnacho <carlosg@gnome.org> Date: Wed Jan 14 16:59:36 2015 +0100 widget: Free the controller list on finalize() If this is done on dispose(), the widget may be destroyed (and its controllers list NULLified) within _gtk_widget_run_controllers(), causing warnings/crashes when it just tried to hop on the next controllers. Freeing the controllers here should be a safety net for implementations, so it also makes sense to do this late. The widgets that choose to free their controllers on dispose can still do so, and get _gtk_widget_remove_controller() called for these as an indirect result. This needs to be reconsidered.
Yes, just realized that: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff759f2d3 in _gtk_widget_remove_controller ( widget=widget@entry=0xba2c50, controller=0x7de5f0) at gtkwidget.c:17247 17247 g_return_if_fail (GTK_IS_EVENT_CONTROLLER (controller)); (gdb) bt
+ Trace 234735
One solution could be to have GtkWidget hold a reference to each GtkEventController in the list. After all, widgets can own controllers, and not the other way round.
Created attachment 298222 [details] [review] eventcontroller, widget: Don't crash if destroyed before the other Or, how about a weak reference in both directions? We might need to add a few NULL checks in that case.
Created attachment 298223 [details] [review] eventcontroller, widget: Don't crash if destroyed before the other Fix a typo in the commit message.
Created attachment 298224 [details] [review] eventcontroller: Chain up on constructed
The patches worked for me. Gnome-documents no longer crashes.
Comment on attachment 298223 [details] [review] eventcontroller, widget: Don't crash if destroyed before the other Looks good to me, I guess the oddity comes from gtk_gesture*_new() "attaching" the gesture to the widget, but also returning ref_count=1 on the controller, I can see how that confuses bindings... Using weak refs in this scenario looks proper.
Comment on attachment 298224 [details] [review] eventcontroller: Chain up on constructed Looks good, the GObject contructed vmethod is blank, but better to just upchain indeed.
Thanks, Carlos.