After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 683376 - Make it easier to implement async methods that return non-ref counted types
Make it easier to implement async methods that return non-ref counted types
Status: RESOLVED DUPLICATE of bug 661767
Product: glib
Classification: Platform
Component: gio
unspecified
Other All
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-09-05 00:04 UTC by Debarshi Ray
Modified: 2012-09-05 00:28 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Debarshi Ray 2012-09-05 00:04:14 UTC
Currently there is a dark corner in implementing GIO style async methods which return a pointer to non-ref counted types (eg., GList *) and transfer ownership. Since _finish is optional, the async implementation must set a GDestroyNotify while setting the return value in the op_res so that it is not leaked if the _finish is never invoked. However, if you do that, you can not transfer ownership to the client in the _finish because you can not mask the GDestroyNotify once it is set.

People get around this by putting the actual result (eg., GList *) in a wrapper structure and place that wrapper inside the op_res_gpointer, and provide a GDestroyNotify that skips destroying the actual result within the wrapper if that field is NULL. This is effectively yanking out the GDestroyNotify from the GSimpleAsyncResult.

This way of yanking out the GDestroyNotify is a clever trick, but looks like a hack because the GIO documentation never gives you a hint about the loop hole.

Often this wrapper is re-used to carry the async's internal context. But, if your async method is simple enough that you don't need any internal context outside the instance's priv, you are still stuck with a wrapper around your result just for this "yank out the GDestroyNotify" trick.

Moreover, advocating that the internal context be shoehorned with the result of the operation as the idiomatic way of implementing async methods undermines what the documentation says about the g_simple_async_result_set_op_res* methods. They are labelled as a way of setting the "operation result", not random "user data" or context.

So, as a first step we should make this bit about returning non-ref counted types more obvious in the documentation.

Plus, we could introduce something like a g_simple_async_result_take_op_res_gpointer which can be used in the _finish to steal the reference to the pointer and pass it to the client by unsetting the GDestroyNotify.

IMHO, this lets you use a cleaner idiom, where the op_res is only used for the return value. If the async needs its own private context then it can create the GSimpleAsyncResult with its own wrapper callback / user_data that essentially wrap the client's callback / user_data.

I have seen cases in the wild where people naively use this second idiom because that looks obvious from reading the docs for the first time, until somewhere down the line they learn the trick and realize that they were not handling the optional _finish case well.
Comment 1 Dan Winship 2012-09-05 00:27:49 UTC
GTask, the GSimpleAsyncResult replacement which will be landing just post-2.34, solves this; its equivalent of _get_op_res_pointer() steals the result, as you suggest.

*** This bug has been marked as a duplicate of bug 661767 ***
Comment 2 Dan Winship 2012-09-05 00:28:20 UTC
(see the wip/task branch in git)