GNOME Bugzilla – Bug 644757
Support GObject.Closure callbacks
Last modified: 2011-03-21 16:07:36 UTC
Here is a recreation of the conversation I had on IRC with tomeo. <adamplumb> I'm working on updating the nautilus-python bindings to work with the Nautilus gir annotations <adamplumb> I've got a problem where I'm passing a GObject.Closure from the C extension to a python extension, which calls a gir-provided method "Nautilus.info_provider_update_complete_invoke". I get the following TypeError from /gi/types.py:40, argument 0 must be callable, not Closure <adamplumb> In python, I see that the type of the parameter is <class 'gi.repository.GObject.Closure'> The problem is that pygi-argument.c (http://git.gnome.org/browse/pygobject/tree/gi/pygi-argument.c#n173) will not allow a GObject.Closure to pass the check because it is not callable. It was not decided in IRC what the solution would be. Either we could update that check to account for GObject.Closure types or make the GObject.Closure type callable. FYI, This is blocking me from releasing an updated nautilus-python that works with gobject-introspection.
GObject.Closure shouldn't quite be exposed directly. g_closure_invoke() can't be supported directly by gi, I guess we need python bindings to that, similar to what pygobject does for signal callbacks. It'll be a bit of work to support that in pygi.
Make it a foreign type and create a wrapper that makes it executable. This would also be nice if we did this for callbacks too. There are some events like gtk_widget_forall, which send in a callback that should be called by the python code. This is for instance used by show_all, hide_all, etc. to make sure all children of a container also get shown or hidden.
I know it is hard to predict when things will get done in open source projects, but is this likely to be done for the next bugfix release or will it have to wait for a major release? I would be happy to help test this if someone were to produce a patch.
As this blocks the nautilus python plugin we should make it a priority. A quick fix is to just check the type first to see if it is already a closure. You just need to pass a closure correct? You don't need to call it do you?
That is correct. The closure in question is passed to and from the python extension, it does not need to be called from python itself.
Created attachment 183573 [details] [review] [gi] marshal raw closures * before we were able to marshal python callables into methods that took GClosures but we had no way to take a GClosure returned from one method and pass it to another - this enables that usecase
Please try out the patch above. This should fix the immediate issue of passing GClosures from one C function to another through GI. As for making them executable, which I think we should do, we need to create a new wrapper for Closures. Right now they are wrapped as structs. I would say do that on master as it isn't really important for the current stable branch.
Note that the attached patch needs a newer GI for tests to pass but will run fine on the latest released version of GI.
Created attachment 183585 [details] backtrace I'm not sure yet if I'm doing something wrong, but I've patched the latest pygobject code with your patch, built it (with a bunch of warnings), and updated my PKG_CONFIG_PATH and PYTHONPATH environment variables to point to the git version of pygobject. FYI, my development machine is Fedora 15 running GNOME 3. When running a python extension that passes the closure, I now get a segmentation fault. I've attached the backtrace.
FYI, the nautilus-3.0 branch of nautilus-python is available at http://git.gnome.org/browse/nautilus-python/log/?h=nautilus-3.0 and you can access the python extension I'm using at http://git.gnome.org/browse/nautilus-python/tree/examples/update-file-info-async.py?h=nautilus-3.0.
Make sure you compile the pygobject-2-28 branch. Master should work but I haven't been running it.
Oh, I looked at your example and I am pretty sure your closure is destroyed before then. Try doing it without the timeout to see if this is the issue.
Yes, if I call the Nautilus.info_provider_update_complete_invoke() method from within the update_file_info method without a timeout then I don't get a segmentation fault. You're right, the closure pointer must be getting destroyed. FYI, this pointer should not be getting destroyed since the whole point of it is that it be passed to the update_complete_invoke method asynchronously.
It then needs to be annotated as async which will destroy the callback after it is called once. There still can be an issue with refcounting in PyGObject.
Created attachment 183608 [details] [review] [gi] marshal raw closures * before we were able to marshal python callables into methods that took GClosures but we had no way to take a GClosure returned from one method and pass it to another - this enables that usecase
Attached is a new patch which uses the correct pyg_boxed_get instead of pyg_pointer_get. This might fix the lifecycle issue. However, looking at the nautilus-python code in the 3.0 branch one red flag jumps out at me in that you are calling update_file_info_full from a static binding and wrapping it with your own python object. We wrap our closures as PyGIBoxed objects which are derived from PyGBoxed objects. If you aren't wrapping with at least PyGBoxed then there is no way for this to work. In fact getting rid of any static bindings and going fully GI will ensure you don't run into any incompatibilities.
Review of attachment 183608 [details] [review]: Looks great, do you have a bug already about making those wrapped GClosures callable?
I got the new patch working with some changes of my own. Regarding comment #16, I don't use any static bindings for nautilus-python any more. Instead, I check that python scripts in certain directories are importing the Nautilus module from the gi repository. I still need to create a Nautilus C extension that acts as the go-between.
@Adam, can you attach your edited patch? or do you mean you made changes in the nautilus-plugins? @Tomeu: Not yet but it would be extremely easy by creating a new PyGBoxed type to wrap GClosures. I do have a bug for callbacks - Bug #644926 - but that is a much harder issue (and much more important due to gtk_container_forall being the only way to really support subclassing with internal children)
hi John, sorry I meant changes in nautilus-python. Your latest patch is fine.
I'm not committing this yet, I am going to make a PyGICClosure type that is callable. Adam, can you check it once I post the patch later today?
Sure no problem.
Ack, wasn't able to get it working correctly. I'm not sure how gi.repository.GObject.Closure becomes a subtype of PyGIBoxed. I created a PyGIBoxedClosure object but can't get gi.repository.GObject.Closure to derive from it (it does run through the init code though. I'll keep working on it.
Ok got it working but it turns out we can't safely invoke closures since we have no GI info on them. I'm going to apply the patch we have here.