Fail gracefully on bad TreePath

[v1993]

Turned out that my #161 may lead to crashes under some conditions. This patch fixes that. For example, I got crash in samples/gtk-demo/main.lua:102.

Note: gtk warning still gets printed to console. I'm not really sure how much of a bug is this, but still better avoid crashing.

[psychon]

I got crash in samples/gtk-demo/main.lua:102

Huh? https://github.com/pavouk/lgi/blob/6e392a086b2490613a31a21ad8e7782b868c2c05/samples/gtk-demo/main.lua#L100-L111

First I thought "there should be a test for this; something that previously crashes/fails and then is fixed by the new commits". Then I thought, "well, gtk-demo could serve as this test, couldn't it?". Now I'm unsure if I understand your comment correctly.

Could you add some test for this to the test suite or tell me why that is not necessary?

[v1993]

I'm not really into your test suite (are there even docs for it?), but having a test sounds like a good idea anyways.

So bug happens when we try to invoke method on treemodel. I'm surprised it wasn't covered by tests as well.

[psychon]

I'm not really into your test suite

Well, you just add a new method that does something and causes a Lua error (or crash) if things go wrong. Something like:

diff --git a/tests/gtk.lua b/tests/gtk.lua
index 62464e2..65b1983 100644
--- a/tests/gtk.lua
+++ b/tests/gtk.lua
@@ -394,3 +394,8 @@ function gtk.actiongroup_index()
    check(ag.action.a1 == a1)
    check(ag.action.a2 == a2)
 end
+
+function gtk.tree_path()
+    local Gtk = lgi.Gtk
+    error("I do not actually know what to do here")
+end

(are there even docs for it?),

No idea, sorry.

[v1993]

@psychon This looks kinda interesting. Seems that bug happens only with TreeModelSort but not with liststore or treestore. While this patch fixes crash, other things look a bit off.

If I index TreeStore or ListStore, members are checked first and only then override is invoked. On the other hand, TreeModelSort always call override first and only then check class members, so warning will always be printed when you call method. This difference looks like a (separate) bug for me.

Regardless of this, this PR fixes error happening in a wrong place and under some strange conditions.

What do you think of this test?

function gtk.treemodelsort_method()
   local Gtk = lgi.Gtk
   -- Shouldn't error when making TreePath, only print warning
   local noop = lgi.Gtk.TreeModelSort().set_sort_func
end

[psychon]

While this patch fixes crash, other things look a bit off.

Seems like the test suggestion already worked. :-)

What do you think of this test?

By itself, it is a bad test. However, since you say this actually breaks currently, it is good enough.

(Also, I do not know much about GTK or GObject, so do not expect too much from me.)

[v1993]

@psychon Well, I was simply investigating why this problem doesn't happen with ListStore or TreeStore. To trigger it there you need to index non-existing method, but it happens on every single indexing of TreeModelSort because there override is called first and only after it methods are checked.

Should I open an issue about this?

[v1993]

Last commit adds test, sorry for default description.

[v1993]

@psychon Well, there's a HUGE problem with your test suite! Just ran it manually. Results? 4 errors turned into Lgi-WARNINGs, including one directly indicating very this problem fixed by current pull request:

cd .. && LD_LIBRARY_PATH=tests:$LD_LIBRARY_PATH \
    GI_TYPELIB_PATH=tests:$GI_TYPELIB_PATH \
    LUA_PATH="./?.lua;/home/v/.luarocks/share/lua/5.3/?.lua;/home/v/.luarocks/share/lua/5.3/?/init.lua;/usr/local/share/lua/5.3/?.lua;/usr/local/share/lua/5.3/?/init.lua;/usr/local/lib/lua/5.3/?.lua;/usr/local/lib/lua/5.3/?/init.lua;/usr/share/lua/5.3/?.lua;/usr/share/lua/5.3/?/init.lua;./?.lua;./?/init.lua;" \
    LUA_CPATH="./?.so;/home/v/.luarocks/lib/lua/5.3/?.so;/usr/local/lib/lua/5.3/?.so;/usr/lib/x86_64-linux-gnu/lua/5.3/?.so;/usr/lib/lua/5.3/?.so;/usr/local/lib/lua/5.3/loadall.so;./?.so;" \
    /usr/bin/dbus-run-session lua5.3 tests/test.lua

(process:23507): Lgi-WARNING **: 18:57:49.448: Error raised while calling 'lgi.cbk (number): Regress': attempt to call a number value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (string): Regress': attempt to call a string value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (table: 0x55791f2a0ab0): Regress': attempt to call a table value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (table: 0x55791f11cb60): Regress': attempt to call a table value
gireg   : all 114 tests passed.
marshal : all 1 tests passed.
corocbk : all 3 tests passed.
record  : all 1 tests passed.
dbus-daemon[23506]: [session uid=1000 pid=23506] Activating service name='org.gtk.vfs.Daemon' requested by ':1.0' (uid=1000 pid=23507 comm="lua5.3 tests/test.lua ")
dbus-daemon[23506]: [session uid=1000 pid=23506] Successfully activated service 'org.gtk.vfs.Daemon'
fusermount: failed to access mountpoint /run/user/1000/gvfs: Permission denied
gobject : all 18 tests passed.
glib    : all 6 tests passed.
variant : all 18 tests passed.
dbus    : all 3 tests passed.

(lua5.3:23507): Gtk-WARNING **: 18:57:50.148: ../../../../gtk/gtktreemodel.c:645: Invalid path set_sort_func passed to gtk_tree_path_new_from_string
gtk     : all 22 tests passed.
cairo   : all 15 tests passed.
pango   : all 1 tests passed.
gio     : all 2 tests passed.
A connection to the bus can't be made
cd .. && LD_LIBRARY_PATH=tests:$LD_LIBRARY_PATH \
    GI_TYPELIB_PATH=tests:$GI_TYPELIB_PATH \
    LUA_PATH="./?.lua;/home/v/.luarocks/share/lua/5.3/?.lua;/home/v/.luarocks/share/lua/5.3/?/init.lua;/usr/local/share/lua/5.3/?.lua;/usr/local/share/lua/5.3/?/init.lua;/usr/local/lib/lua/5.3/?.lua;/usr/local/lib/lua/5.3/?/init.lua;/usr/share/lua/5.3/?.lua;/usr/share/lua/5.3/?/init.lua;./?.lua;./?/init.lua;" \
    LUA_CPATH="./?.so;/home/v/.luarocks/lib/lua/5.3/?.so;/usr/local/lib/lua/5.3/?.so;/usr/lib/x86_64-linux-gnu/lua/5.3/?.so;/usr/lib/lua/5.3/?.so;/usr/local/lib/lua/5.3/loadall.so;./?.so;" \
    tests/test_c
Success

You need to catch errors better there…

[psychon]

The following are issue #185:

(process:23507): Lgi-WARNING **: 18:57:49.448: Error raised while calling 'lgi.cbk (number): Regress': attempt to call a number value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (string): Regress': attempt to call a string value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (table: 0x55791f2a0ab0): Regress': attempt to call a table value

(process:23507): Lgi-WARNING **: 18:57:49.449: Error raised while calling 'lgi.cbk (table: 0x55791f11cb60): Regress': attempt to call a table value

I'm not seeing the following when I ran the test suite myself locally:

(lua5.3:23507): Gtk-WARNING **: 18:57:50.148: ../../../../gtk/gtktreemodel.c:645: Invalid path set_sort_func passed to gtk_tree_path_new_from_string
gtk     : all 22 tests passed.

[v1993]

Well that's strange… Do you have error if you run

= (require 'lgi').Gtk.TreeModelSort().set_sort_func

in Lua interactive console?

[psychon]

With the current LGI version from Debian testing:

$ for lua in lua5.1 lua5.2 lua5.3 luajit ; do echo ; echo $lua ; $lua -e 'print(require("lgi").Gtk.TreeModelSort().set_sort_func)' ; done

lua5.1

(lua5.1:3791): dbind-WARNING **: 18:31:02.736: AT-SPI: Error retrieving accessibility bus address: org.freedesktop.DBus.Error.ServiceUnknown: The name org.a11y.Bus was not provided by any .service files
lgi.fun (0x7fb0d91cb170): Gtk.TreeSortable.set_sort_func

lua5.2

(lua5.2:3792): dbind-WARNING **: 18:31:02.770: AT-SPI: Error retrieving accessibility bus address: org.freedesktop.DBus.Error.ServiceUnknown: The name org.a11y.Bus was not provided by any .service files
lgi.fun (0x7f6f824a7170): Gtk.TreeSortable.set_sort_func

lua5.3

(lua5.3:3793): dbind-WARNING **: 18:31:02.805: AT-SPI: Error retrieving accessibility bus address: org.freedesktop.DBus.Error.ServiceUnknown: The name org.a11y.Bus was not provided by any .service files
lgi.fun (0x7fbbf2983170): Gtk.TreeSortable.set_sort_func

luajit

(luajit:3794): dbind-WARNING **: 18:31:02.836: AT-SPI: Error retrieving accessibility bus address: org.freedesktop.DBus.Error.ServiceUnknown: The name org.a11y.Bus was not provided by any .service files
lgi.fun (0x7fb2f6816170): Gtk.TreeSortable.set_sort_func

With LGI git and the following patch:

diff --git a/tests/gtk.lua b/tests/gtk.lua
index 62464e2..0fc3ebc 100644
--- a/tests/gtk.lua
+++ b/tests/gtk.lua
@@ -393,4 +393,8 @@ function gtk.actiongroup_index()
    local ag = Gtk.ActionGroup { a1, a2 }
    check(ag.action.a1 == a1)
    check(ag.action.a2 == a2)
+
+   -- Without this patch nothing does this indexing, right? Or am I missing
+   -- something?
+   print(Gtk.TreeModelSort().set_sort_func)
 end

(lua:4313): Gtk-WARNING **: 18:32:52.534: ../../../../gtk/gtktreemodel.c:657: Invalid path set_sort_func passed to gtk_tree_path_new_from_string
gtk     : 21:actiongroup_index                  FAIL
             ./lgi/override/Gtk.lua:318: bad argument #2 to 'get_iter' (Gtk.TreePath expected, got nil)
stack traceback:
    [C]: in function 'get_iter'
    ./lgi/override/Gtk.lua:318: in function '_element'
    ./lgi/override/GObject-Object.lua:245: in function 'inherited_gobject_element'
    ./lgi/override/Gio.lua:190: in function '_element'
    ./lgi/class.lua:143: in function 'inherited_class_element'
    ./lgi/override/Gio.lua:179: in function 'inherited_element'
    ./lgi/override/GObject-Object.lua:233: in function 'inherited_gobject_element'
    ./lgi/override/Gio.lua:190: in function '_element'
    ./lgi/component.lua:158: in function <./lgi/component.lua:156>
    [C]: in function '__index'
    tests/gtk.lua:399: in function <tests/gtk.lua:390>
    [C]: in function 'xpcall'
    tests/test.lua:50: in function 'runfunc'
    tests/test.lua:73: in function 'run'
    tests/test.lua:141: in main chunk
    [C]: in ?
gtk     : FAILED 1 of 21 tests

[v1993]

Well, part of mystery just got solved: I've been testing on version which include my new test, sorry…

However, on main repo's git head I get

v@v-home:~/Downloads$ lua
Lua 5.3.3  Copyright (C) 1994-2016 Lua.org, PUC-Rio
> = (require 'lgi').Gtk.TreeModelSort().set_sort_func

(lua:28390): Gtk-WARNING **: 19:38:01.894: ../../../../gtk/gtktreemodel.c:645: Invalid path set_sort_func passed to gtk_tree_path_new_from_string
/home/v/.luarocks/share/lua/5.3/lgi/override/Gtk.lua:318: bad argument #2 to 'get_iter' (Gtk.TreePath expected, got nil)
stack traceback:
    [C]: in method 'get_iter'
    /home/v/.luarocks/share/lua/5.3/lgi/override/Gtk.lua:318: in method '_element'
    .../.luarocks/share/lua/5.3/lgi/override/GObject-Object.lua:245: in upvalue 'inherited_gobject_element'
    /home/v/.luarocks/share/lua/5.3/lgi/override/Gio.lua:190: in method '_element'
    /home/v/.luarocks/share/lua/5.3/lgi/class.lua:143: in upvalue 'inherited_class_element'
    /home/v/.luarocks/share/lua/5.3/lgi/override/Gio.lua:179: in upvalue 'inherited_element'
    .../.luarocks/share/lua/5.3/lgi/override/GObject-Object.lua:233: in upvalue 'inherited_gobject_element'
    /home/v/.luarocks/share/lua/5.3/lgi/override/Gio.lua:190: in method '_element'
    /home/v/.luarocks/share/lua/5.3/lgi/component.lua:158: in function </home/v/.luarocks/share/lua/5.3/lgi/component.lua:156>
    [C]: in metamethod '__index'
    stdin:1: in main chunk
    [C]: in ?

And on my:

v@v-home:~/Downloads$ lua
Lua 5.3.3  Copyright (C) 1994-2016 Lua.org, PUC-Rio
> = (require 'lgi').Gtk.TreeModelSort().set_sort_func

(lua:27798): Gtk-WARNING **: 19:37:45.821: ../../../../gtk/gtktreemodel.c:645: Invalid path set_sort_func passed to gtk_tree_path_new_from_string
lgi.fun (0x7f6ee02501e0): Gtk.TreeSortable.set_sort_func

[v1993]

@psychon Ah, I think I got this. Debian testing most likely use last release which lack my patch that introduced this problem. As a sidenote, you really should make new release sometime (but review this and #228 first please).

[psychon]

you really should make new release sometime

I cannot, only @pavouk has access to The Force.

[v1993]

@psychon So, what about this patch?

[v1993]

@psychon Oh, thank you. Can you take a look at #234 too, please?

[psychon]

I only have finite speed. ;-) I am already looking.