In spi_object_return_reference() inside at-spi2-atk/atk-adaptor/object.c, an extra g_object_unref() done by the end of the function is causing problems when the AtkObject doesn't have an extra reference (that is, it's just the same reference held internally by the implementation of ATK in use: GAIL or a11y layer in WebKitGtk, for instance), specially when a leasing is needed since the wouldn't enter in the cache. An example:
Suppose the AtkObject in use has a GObject ref_count of 1 when calling to spi_object_return_reference() (that is, it doesn't come from an atk_*_ref_*() method, but from a atk_*_get_*() one):
1. spi_object_append_reference() is called for the object, which calls to spi_object_lease_if_needed().
2. spi_object_lease_if_needed() determines that a leasing is needed (maybe the AtkObject is about a cell in a table), so it calls to spi_leasing_take().
3. spi_leasing_take() increments the ref_count to 2 and sets an expiry function to be called after the timeout (expiry_func()).
4. When the control returns to spi_object_return_reference(), g_object_unref() is called over the AtkObject, as it's not NULL, decreasing its ref_count to 1.
5. After some time, the expiry_func function gets called, decreasing the ref_count to 0 and therefore getting the AtkObject destroyed, when that's something that shouldn't happen since that object would be property of the ATK implementation being used, such as GAIL for Gtk or the a11y implementation for WebKitGtk+.
Hence, it would be needed to take into account whether an AtkObject comes from an atk_*_ref_*() or an atk_*_get_*() method when calling to spi_object_return_reference(), to know when the extra g_object_unref() would be needed or not. This way, we wouldn't run into problems further, specially when leasing is involved, as you can see in the example above.
Created attachment 36414 [details] [review]
Implemented patch proposal as suggested by Mike Gorse: adding an extra parameter to decide whether to unref in spi_object_return_reference()
Committed. Thanks for the patch.
Reopening after thinking that perhaps the implementation of the attached patch was not the best one:
I think that, instead of adding an extra parameter to know when to unref the AtkObject inside spi_object_return_reference(), it would be better just to completely remove the g_object_unref() from that function and only do it, when needed, after calling it, that is, from the callers.
I should have noticed before sending the former patch, sorry for the annoyance. I'm attaching a new patch right now...
Created attachment 36433 [details] [review]
Patch proposal (a better one)
Here is the new patch, which applies cleanly right on top of the last one, that is, on top of current master :-)