gbinding: use g_object_weak_ref_full for bindings

The pattern here is that we acquire strong references via GWeakRef.
So you might think that g_object_weak_unref() is safe to call.

However, it is not safe, in case where another thread might run
g_object_run_dispose(). Which is clear, because GWeakRef only ensures we
hold a strong reference, but g_object_run_dispose() is anyway run on an
object where we already hold a reference.

In weak_unbind(), we obtain strong references, but (after) that
point, another thead might call g_object_run_dispose(). Then,
inside unbind_internal_locked() we do:

          g_object_weak_unref (source, weak_unbind, context);
          binding_context_unref (context);

Note that here weak_unbind might have already be unrefed, and
g_object_weak_unref() fails an assertion. But worse, the
weak_unbind callback will also be called and we issue two
binding_context_unref() and crash.

This is fixed by using g_object_weak_ref_full() (which handles the case
that the weak notification might have already be emitted) and a separate
GDestroyNotify (that is guaranteed to run exactly once).

This still doesn't make it fully work. Note that we also call

  g_signal_handler_disconnect (source, binding->source_notify);

this has exactly the same problem. A concurrent g_object_run_dispose()
will already disconnect all signal handlers, and calling disconnect
fails an assertion. I think the solution for that is a new API
g_signal_handler_try_disconnect(), which does not assert. After all, the
gulong signal ID is unique (the gulong is large enough to never wrap and
there is even a g_error() check against that).
This commit is contained in:
Thomas Haller
2025-04-25 08:56:55 +02:00
parent 78a298d526
commit 38f40d3cce
2 changed files with 13 additions and 11 deletions

View File

@@ -314,8 +314,7 @@ unbind_internal_locked (BindingContext *context, GBinding *binding, GObject *sou
{
g_signal_handler_disconnect (source, binding->source_notify);
g_object_weak_unref (source, weak_unbind, context);
binding_context_unref (context);
g_object_weak_unref_full (source, weak_unbind, context, FALSE, FALSE);
binding->source_notify = 0;
}
@@ -341,8 +340,7 @@ unbind_internal_locked (BindingContext *context, GBinding *binding, GObject *sou
/* Remove the weak notify from the target, at most once */
if (binding->target_weak_notify_installed)
{
g_object_weak_unref (target, weak_unbind, context);
binding_context_unref (context);
g_object_weak_unref_full (target, weak_unbind, context, FALSE, FALSE);
binding->target_weak_notify_installed = FALSE;
}
}
@@ -376,7 +374,6 @@ weak_unbind (gpointer user_data,
if (!binding)
{
/* The binding was already destroyed before so there's nothing to do */
binding_context_unref (context);
return;
}
@@ -430,9 +427,6 @@ weak_unbind (gpointer user_data,
/* This will take care of the binding itself. */
if (binding_was_removed)
g_object_unref (binding);
/* Each weak notify owns a reference to the binding context. */
binding_context_unref (context);
}
static gboolean
@@ -833,7 +827,11 @@ g_binding_constructed (GObject *gobject)
source_notify_closure,
FALSE);
g_object_weak_ref (source, weak_unbind, binding_context_ref (binding->context));
g_object_weak_ref_full (source,
weak_unbind,
binding_context_ref (binding->context),
(GDestroyNotify) binding_context_unref,
FALSE);
if (binding->flags & G_BINDING_BIDIRECTIONAL)
{
@@ -853,7 +851,11 @@ g_binding_constructed (GObject *gobject)
if (target != source)
{
g_object_weak_ref (target, weak_unbind, binding_context_ref (binding->context));
g_object_weak_ref_full (target,
weak_unbind,
binding_context_ref (binding->context),
(GDestroyNotify) binding_context_unref,
FALSE);
/* Need to remember separately if a target weak notify was installed as
* unlike for the source it can exist independently of the property

View File

@@ -3966,8 +3966,8 @@ _g_object_weak_ref (GObject *object,
* @object: #GObject to remove a weak reference from
* @notify: callback to search for
* @data: data to search for
* @synchronize: XXX
* @destroy: an optional #GDestroyNotify.
* @synchronize: XXX
*
* Adds a weak reference callback to an object. Weak references are
* used for notification when an object is disposed. They are called