GNOME Bugzilla – Bug 669496
Array argument to Python signal callbacks cause crash
Last modified: 2018-01-10 20:13:57 UTC
I saw the following g-i commit: http://git.gnome.org/browse/gobject-introspection/commit/?id=6757460e4a95f818d3a73d7904ef8a7afb6987fc I do not think pygobject handles it. Attached there is a very quick unit test which indeed does not pass (though it may also the unit test being buggy, it was a 3 minutes hack just to check if array callbacks were already working)
Created attachment 206923 [details] unit test showing the problem
It's indeed not working well. The test case needs to be fixed a bit, though: test_callback_array_arg (test_everything.TestCallbacks) ... TypeError: callback() takes exactly 2 arguments (4 given) When the callback signature gets changed to actually take the two length arguments def callback(ints, ints_len, strings, strings_len): it gets a little further: AssertionError: Lists differ: [] != [-1, 0, 1, 2] So the array arguments indeed do not get marshalled correctly. Finally, I think the test case should also check the return value of test_array_callback(). It's supposed to return the sum of the two callback() returns, which is 1 the first time and 2 the second. This shows that the return value is utter bogus: AssertionError: 2038473632 != 3
Created attachment 211298 [details] test case Updated test case. Not marking as patch, to get this bug off the patch review queue, and it is not a solution for this bug.
Created attachment 237518 [details] [review] test case for signal While debugging bug 693994 I noticed a similar problem: signals which receive a GArray argument do not work. They fail with ** (runtests.py:22846): CRITICAL **: Stack overflow protection. Can't copy array element into GIArgument. that's because _pygi_argument_to_object() is being called with an array of GValues, not an array of uints as it would expect. I just wanted to put this new test case somewhere.
Comment on attachment 211298 [details] test case The issue this test case was showing already seems to be fixed (and a similar test case already exists): https://git.gnome.org/browse/pygobject/tree/tests/test_everything.py?id=3.9.5#n687
Comment on attachment 237518 [details] [review] test case for signal Verified this test still crashes. Marking as a patch again because we can commit it with @unittest.skip and a message pointing to this bug report.
When digging into the code I have found multiple issues when it comes to passing arrays into callbacks and in the case of bug 702508, vfuncs. It seems like a better approach would be to work on bug 727004 instead of trying to fix broken marshaling as a test case already exists for passing arrays into a function and works as expected. Am I correct in my understanding of the state of pygobject in regards to the marshaling code? I would like to fix these issues and any performance boost from using the cache is highly desired. However, I am missing a bit of info on how to unify the two marshaling paths so they both use the cache and avoid pygi-argument.c's marshaling.
I'm afraid this one is going to require work going into bug 726999 as well.
Comment on attachment 237518 [details] [review] test case for signal Committed test case with @unittest.skip
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pygobject/issues/23.