Ensure to sink floating references passed unowned to GetObject() and to not increase their reference count

The API contract between GObject-Introspection and bindings is that
functions returning transfer-none floating references pass a reference
to the bindings that should be taken ownership of by sinking it. Not
doing so is wrong and will lead to memory leaks or double frees.

Previously we would not distinguish this case and simply increment the
reference count. In addition we would then sink the floating reference
when the Object.Raw field is set later for InitiallyUnowned subclasses.

Remove that last part and instead check directly in Object.Raw.set if we
get a floating reference and if so simply sink it here and take
ownership of it. The general assumption of Object.Raw.set is that it
gets passed a reference that it should take ownership of.

So in summary:
1) GetObject() would only increase the reference count of unowned,
   non-floating references so that we own it. For unowned, floating
   references it assumes ownership of the reference.
2) Raw.set assumes ownership of the reference passed to it and if it
   happens to be a floating reference then it will first sink it.

Also warn if we get a floating, owned reference passed to GetObject() as
that case is not allowed by GObject-Introspection and would cause the
reference to be leaked.

This fixes a memory leak with functions returning unowned, floating
references and with functions returning owned, non-floating references
of InitiallyUnowned subclasses. And at the same time keeps constructors
for InitiallyUnowned subclasses working correctly without leaks.

See https://gitlab.freedesktop.org/gstreamer/gstreamer-sharp/issues/31
This commit is contained in:
Sebastian Dröge 2020-01-28 12:50:17 +02:00 committed by Harry
parent 5b63473a1c
commit 89cfd16c10
2 changed files with 39 additions and 21 deletions

View file

@ -5,7 +5,7 @@
// Copyright (c) 2004-2005 Novell, Inc. // Copyright (c) 2004-2005 Novell, Inc.
// //
// This program is free software; you can redistribute it and/or // This program is free software; you can redistribute it and/or
// modify it under the terms of version 2 of the Lesser GNU General // modify it under the terms of version 2 of the Lesser GNU General
// Public License as published by the Free Software Foundation. // Public License as published by the Free Software Foundation.
// //
// This program is distributed in the hope that it will be useful, // This program is distributed in the hope that it will be useful,
@ -38,26 +38,19 @@ namespace GLib {
} }
} }
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate void d_g_object_ref_sink(IntPtr raw); delegate void d_g_object_ref_sink(IntPtr raw);
static d_g_object_ref_sink g_object_ref_sink = FuncLoader.LoadFunction<d_g_object_ref_sink>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_ref_sink")); static d_g_object_ref_sink g_object_ref_sink = FuncLoader.LoadFunction<d_g_object_ref_sink>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_ref_sink"));
protected override IntPtr Raw { [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
get {
return base.Raw;
}
set {
if (value != IntPtr.Zero)
g_object_ref_sink (value);
base.Raw = value;
}
}
delegate bool d_g_object_is_floating(IntPtr raw); delegate bool d_g_object_is_floating(IntPtr raw);
static d_g_object_is_floating g_object_is_floating = FuncLoader.LoadFunction<d_g_object_is_floating>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_is_floating")); static d_g_object_is_floating g_object_is_floating = FuncLoader.LoadFunction<d_g_object_is_floating>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_is_floating"));
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate void d_g_object_force_floating(IntPtr raw); delegate void d_g_object_force_floating(IntPtr raw);
static d_g_object_force_floating g_object_force_floating = FuncLoader.LoadFunction<d_g_object_force_floating>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_force_floating")); static d_g_object_force_floating g_object_force_floating = FuncLoader.LoadFunction<d_g_object_force_floating>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_force_floating"));
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate void d_g_object_unref(IntPtr raw); delegate void d_g_object_unref(IntPtr raw);
static d_g_object_unref g_object_unref = FuncLoader.LoadFunction<d_g_object_unref>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_unref")); static d_g_object_unref g_object_unref = FuncLoader.LoadFunction<d_g_object_unref>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_unref"));

View file

@ -8,7 +8,7 @@
// Copyright (c) 2013 Andres G. Aragoneses // Copyright (c) 2013 Andres G. Aragoneses
// //
// This program is free software; you can redistribute it and/or // This program is free software; you can redistribute it and/or
// modify it under the terms of version 2 of the Lesser GNU General // modify it under the terms of version 2 of the Lesser GNU General
// Public License as published by the Free Software Foundation. // Public License as published by the Free Software Foundation.
// //
// This program is distributed in the hope that it will be useful, // This program is distributed in the hope that it will be useful,
@ -68,7 +68,7 @@ namespace GLib {
handle = IntPtr.Zero; handle = IntPtr.Zero;
if (tref == null) if (tref == null)
return; return;
if (disposing) if (disposing)
tref.Dispose (); tref.Dispose ();
else else
@ -92,7 +92,10 @@ namespace GLib {
[UnmanagedFunctionPointer(CallingConvention.Cdecl)] [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate void d_g_object_unref(IntPtr raw); delegate void d_g_object_unref(IntPtr raw);
static d_g_object_unref g_object_unref = FuncLoader.LoadFunction<d_g_object_unref>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_unref")); static d_g_object_unref g_object_unref = FuncLoader.LoadFunction<d_g_object_unref>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_unref"));
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate bool d_g_object_is_floating(IntPtr raw);
static d_g_object_is_floating g_object_is_floating = FuncLoader.LoadFunction<d_g_object_is_floating>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_is_floating"));
public static Object TryGetObject (IntPtr o) public static Object TryGetObject (IntPtr o)
{ {
if (o == IntPtr.Zero) if (o == IntPtr.Zero)
@ -133,15 +136,26 @@ namespace GLib {
return obj; return obj;
} }
if (!owned_ref) bool unexpected_owned_floating = false;
// If we don't get an owned reference here then we need to increase the
// reference count as CreateObject() always takes an owned reference.
// If we get a floating reference passed, however, then we assume that
// we actually own it and have to sink the floating reference, which
// will happen in the setter for Raw later.
if (!owned_ref && !g_object_is_floating(o))
g_object_ref (o); g_object_ref (o);
else if (owned_ref && g_object_is_floating(o))
unexpected_owned_floating = true;
obj = GLib.ObjectManager.CreateObject(o); obj = GLib.ObjectManager.CreateObject(o);
if (obj == null) { if (obj == null) {
g_object_unref (o); g_object_unref (o);
return null; return null;
} }
if (unexpected_owned_floating)
Console.Error.WriteLine ("Unexpected owned floating reference of " + obj.GetType() + " instance. This will be leaked");
return obj; return obj;
} }
@ -457,7 +471,7 @@ namespace GLib {
{ {
IntPtr native_name = GLib.Marshaller.StringToPtrGStrdup (name); IntPtr native_name = GLib.Marshaller.StringToPtrGStrdup (name);
g_object_class_override_property (oclass, property_id, native_name); g_object_class_override_property (oclass, property_id, native_name);
GLib.Marshaller.Free (native_name); GLib.Marshaller.Free (native_name);
} }
[Obsolete ("Use OverrideProperty(oclass,property_id,name)")] [Obsolete ("Use OverrideProperty(oclass,property_id,name)")]
@ -641,6 +655,10 @@ namespace GLib {
delegate IntPtr d_g_object_newv(IntPtr gtype, int n_params, GParameter[] parms); delegate IntPtr d_g_object_newv(IntPtr gtype, int n_params, GParameter[] parms);
static d_g_object_newv g_object_newv = FuncLoader.LoadFunction<d_g_object_newv>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_newv")); static d_g_object_newv g_object_newv = FuncLoader.LoadFunction<d_g_object_newv>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_newv"));
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
delegate void d_g_object_ref_sink(IntPtr raw);
static d_g_object_ref_sink g_object_ref_sink = FuncLoader.LoadFunction<d_g_object_ref_sink>(FuncLoader.GetProcAddress(GLibrary.Load(Library.GObject), "g_object_ref_sink"));
protected virtual void CreateNativeObject (string[] names, GLib.Value[] vals) protected virtual void CreateNativeObject (string[] names, GLib.Value[] vals)
{ {
GType gtype = LookupGType (); GType gtype = LookupGType ();
@ -682,6 +700,13 @@ namespace GLib {
} }
} }
// All references that we get here are assumed to be owned by us. If we
// get a floating reference then we should take ownership of it by
// sinking it.
if (value != IntPtr.Zero && g_object_is_floating(value)) {
g_object_ref_sink(value);
}
handle = value; handle = value;
if (value != IntPtr.Zero) { if (value != IntPtr.Zero) {
tref = new ToggleRef (this); tref = new ToggleRef (this);
@ -689,7 +714,7 @@ namespace GLib {
} }
} }
} }
} }
public static GLib.GType GType { public static GLib.GType GType {
get { return GType.Object; } get { return GType.Object; }
@ -742,10 +767,10 @@ namespace GLib {
System.Collections.Hashtable data; System.Collections.Hashtable data;
public System.Collections.Hashtable Data { public System.Collections.Hashtable Data {
get { get {
if (data == null) if (data == null)
data = new System.Collections.Hashtable (); data = new System.Collections.Hashtable ();
return data; return data;
} }
} }