GNOME Bugzilla – Bug 756134
Segmentation fault on calling g_simple_action_group_add_action with bad action constructor call
Last modified: 2015-11-02 12:53:19 UTC
When calling Gio.SimpleAction() without an argument it will cause a SIGSEGV when added to a Gtk.ApplicationWindow. I know that this is a programming error of whoever does that. I think calling this constructor without an argument should not be possible in the first place. I don't know whether this is an issue in GObject, PyGObject or SimpleAction. Feel free to reassign since you probably know better than me. Example code: #!/usr/bin/python3 import sys from gi.repository import Gtk, Gio class Test(Gtk.Application): def __init__(self): Gtk.Application.__init__(self, application_id="foo.bar", \ flags=Gio.ApplicationFlags.FLAGS_NONE) self.connect("activate", self.activateCb) def activateCb(self, application): window = Gtk.ApplicationWindow() quit_action = Gio.SimpleAction() window.add_action(quit_action) if __name__ == '__main__': application = Test() application.run(sys.argv) Example backtrace: Thread #1 [python3] 13767 [core: 1] (Suspended : Signal : SIGSEGV:Segmentation fault) g_str_hash() at ghash.c:1.871 0x7fffef71e630 g_hash_table_lookup_node() at ghash.c:375 0x7fffef71da63 g_hash_table_lookup() at ghash.c:1.147 0x7fffef71da63 g_simple_action_group_add_action() at gsimpleactiongroup.c:194 0x7fffef00812a ffi_call_unix64() at unix64.S:76 0x7fffef4e2db0 ffi_call() at ffi64.c:525 0x7fffef4e2818 pygi_invoke_c_callable() at 0x7ffff00ff3f4 pygi_function_cache_invoke() at 0x7ffff01010d8 _callable_info_call() at 0x7ffff00f51d5 PyObject_Call() at abstract.c:2.067 0x7ffff79adf31
Just don't do it if it crashes - g_simple_action_new() takes arguments for a reason...
I know that this is wrong. But shouldn't something (pygobject, gobject) prevent the user from doing so? Shouldn't it check that python code calling Gtk/Glib functions (including constructors) must respect the number and type of parameters?
Created attachment 312827 [details] [review] gapplication: reject actions without names > Just don't do it if it crashes - g_simple_action_new() takes arguments for a > reason... I disagree. This is python, not C. Let's print a critical when creating a GSimpleAction without a name and when trying to add an action without a name to GSimpleActionGroup (which is used by GApplication). The patch also contains a test case.
Review of attachment 312827 [details] [review]: ::: gio/gsimpleactiongroup.c @@ +193,2 @@ action_name = g_action_get_name (action); + g_return_if_fail (action_name != NULL); I would not use a g_return_if_fail() for a state consistency check, here, considering that it can easily be disabled — or, at least, more easily disabled than an assertion. Either we use a critical warning that explains better what you have to do, e.g.: if (action_name == NULL) { g_critical ("The supplied action has no name. You must set the " "GAction:name property when creating an action."); return; } Or use a g_assert().
Created attachment 314629 [details] [review] gapplication: reject actions without names Good point, thanks. I've changed it to print the critical warning you suggested.
Review of attachment 314629 [details] [review]: Looks good to me.
Comment on attachment 314629 [details] [review] gapplication: reject actions without names Attachment 314629 [details] pushed as ee718d3 - gapplication: reject actions without names