Upcoming changes to floating reference handling in GIT master / 1.14

[Sebastian Dröge]

Hi everybody,

This is just to make you all aware of some changes that are going to
happen in GIT master in a little bit, which might affect your code but
hopefully only in a positive way.

Especially testing by users of GObject-Introspection based bindings
(e.g. Python, GJS, ...) would be useful to see if all cases are covered
now.

For the very long version & patches, please read
  https://bugzilla.gnome.org/show_bug.cgi?id=782499
and all the linked bugs.

For background on floating reference please read
  https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#floating-ref


tl;dr is: Things changed, you probably do not have to change any of
your C code and bindings should leak less memory.


Longer summary of the 3 changes is:


1) GObject-Introspection annotation changes – No effect for C developers
========================================================================

Functions that are taking a reference to any object and ref_sink() it,
are now marked as "transfer floating" (an alias for "transfer none")
instead of "transfer full".

The effect of such functions is that they will take ownership of the
reference *if* it was floating, but otherwise increase the reference
count.

This change is needed by GObject-Introspection based bindings to
prevent memory leaks.

See https://bugzilla.gnome.org/show_bug.cgi?id=702960

Affected functions:
  gst_element_add_pad()
  gst_element_add_pad_template()
  gst_bin_add() and related
  gst_device_provider_device_add()
  gst_object_set_parent()
  gst_object_add_control_binding()
  gst_registry_add_plugin()
  gst_registry_add_feature()
  

2) Consistent reference handling in error cases
===============================================

Previously many functions in 1) were doing what was described above in
the normal, non-error case. But in the error case they were doing
nothing with the reference, or something different.

This inconsistency is a problem for bindings as they can't possibly
know what is happening inside the function. This again causes memory
leaks.

It is also confusing for C developers as it is also not how it is
described in the API reference. Most of the C code I found related to
this was wrong and works better with the changes.

As a result, the functions mentioned in 1) are now also calling
ref_sink() on the passed reference in error cases and the unref() it
again immediately. This means that if a floating reference was passed,
the reference count will be decreased by one, in other cases the
reference count effectively stays the same.

This means that if you don't hold another reference (reference count is
1) to a floating reference and pass it to any of these functions, and
it fails, the reference will be invalid afterwards and must not be used
further for anything.

See https://bugzilla.gnome.org/show_bug.cgi?id=747990

3) Don't generate floating references for types that have no "owner"
====================================================================

Everything that is inheriting from GstObject uses floating references,
but for most subclasses this makes no sense as they don't have any/a
single owner, i.e. a place that is calling ref_sink() on them.

This especially affects:
  GstAllocator
  GstBus
  GstClock
  GstBufferPool
  GstControlSource
  GstTask
  GstTaskPool
  ... and all their subclasses.

Previously we removed the floating flag for GstBus, GstClock, GstTask
and GstTaskPool in instance_init/GObject::constructor. This confuses
bindings (again), as the expectation is that newly created subclasses
of GInitiallyUnowned as created by g_object_new*() have the floating
reference flag set. Also GObject-Introspection assumes that new()
functions of these are "transfer floating" (aka "transfer none"), so
memory leaks would happen again.

As a result, the floating flag is now removed only in the new()
functions and they are marked as "transfer full". This works well with
bindings but requires *all* subclasses of the above types to call
gst_object_ref_sink() in the new() functions, especially those that
previously did this elsewhere already.

Not doing that will not have any effect for C code, unless you
previously called gst_object_ref_sink() on any of these types in your
code outside the constructors. I don't think there is any code doing
that.
For bindings this has the effect that there are no more memory leaks.


See https://bugzilla.gnome.org/show_bug.cgi?id=702960


Let us know if this breaks anything for you, and how. And especially
please test the bindings.

-- 
Sebastian Dröge, Centricular Ltd · http://www.centricular.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/gstreamer-devel/attachments/20170517/75ea25de/attachment.sig>