GNOME Bugzilla – Bug 722899
boxed structures not copied when passed in a callback without transfer
Last modified: 2014-08-18 08:33:05 UTC
If a callback passes a boxced structure without giving ownership to the callback (ie, uses transfer none), then pygobject does not copy the boxed structure when needed. Will attach a test case.
See https://bugzilla.gnome.org/show_bug.cgi?id=695056
Created attachment 267122 [details] [review] Add test to gimarshallingtests
Created attachment 267123 [details] [review] pygobject test
Created attachment 267124 [details] [review] fixed pygobject test. Remove a stupid print I had for debugging. Anyway, the test currently fails.
Just a heads up, fixing this will be tricky because it will break some API expectation in use today. Specifically gtk_tree_model_iter_next's 'iter' argument is marked as [in], which is somewhat incorrect. Unfortunately, this sees usage for vfunc implementors which modify the iter input argument to get it to iterate to the next node. https://developer.gnome.org/gtk3/stable/GtkTreeModel.html#gtk-tree-model-iter-next So I think fixing this would break people who rely on the pass a by reference behaviour and fixing the annotation to [inout] would also break the API. This might be a rare case but we either need to special case this methods argument or come up with some other strategie so it continues working.
Review of attachment 267122 [details] [review]: Thanks for the tests. ::: tests/gimarshallingtests.c @@ +4376,3 @@ + GIMarshallingTestsBoxedStruct *box; + +glong Just a thought and I'm not sure it is possible, but it would be nice if we could use a static allocation of the boxed. This way after the callback is run, the value held by the boxed could be tweaked (here in C), then the Python tests could verify its version of the boxed has not been tweaked. This would allow for an "expected failure" on the Python side, instead of just a potentially skipped test.
*** Bug 695056 has been marked as a duplicate of this bug. ***
Created attachment 267255 [details] [review] Improved test (gobject-introspection part) Thanks for the review. I've modified the test to keep the box around and increment its long_ value each time the function is called.
Created attachment 267258 [details] [review] Revised test (pygobject version) This currently fails (returns 2, but it should return 1 if pygobject makes a copy of the struct).
Review of attachment 267255 [details] [review]: looks good to me.
Review of attachment 267258 [details] [review]: Looks good. Feel free to commit this with: @unittest.skipUnless(hasattr(GIMarshallingTests, 'callback_owned_boxed'), 'requires newer version of GI') @unittest.expectedFailure # bug 722899 def test_callback_owned_box(self): ... (I think you can stack those decorators)
Comment on attachment 267258 [details] [review] Revised test (pygobject version) Pushed with the suggested annotations added.
I've verified that some work I'm doing in bug 726999 fixes this issue. There still is the problem mentioned in comment #5 though.