mirror of
https://gitlab.gnome.org/GNOME/glib.git
synced 2025-06-05 20:30:07 +02:00
gobject: preserve weak notifications registered during dispose
We call g_object_weak_release_all() at two places.
Once right before finalize(). At this point, the object is definitely
going to be destroyed, and the user must no longer resurrect it or
subscribe new weak notifications. In that case, we really want to
notify/release all weak notifications.
However, we also call it from g_object_real_dispose(). During dispose,
the API allows the user to resurrect an object. Granted, that is
probably not something anybody should do, but GObject makes a reasonable
attempt to support that.
A possible place to resurrect (and subscribe new weak notifications) is
when GObject calls g_object_real_dispose().
static void
g_object_real_dispose (GObject *object)
{
g_signal_handlers_destroy (object);
/* GWeakNotify and GClosure can call into user code */
g_object_weak_release_all (object);
closure_array_destroy_all (object);
}
But previously, g_object_weak_release_all() would continue iterating
until there are no more weak notifications left. So while the user can
take a strong reference and resurrect the object, their attempts to
register new weak notifications are thwarted.
Instead, when the loop in g_object_weak_release_all() starts, remember
the initial number of weak notifications, and don't release more than
that. Note that WeakRefStack preserves the order of entries, so by
maintaining the "remaining_to_notify" counter we know when to stop.
Note that this brings also an earlier behavior back, where we would call
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
This would take out the entire WeakRefStack at once and notify the weak
notifications registered at the time. But subsequent registrations would
not be released/notified yet.
This commit is contained in:
Thomas Haller
Michael Catanzaro
parent
1312ec6d0b
commit
0be672e1e0
@ -207,7 +207,7 @@ static guint object_floating_flag_handler (GObject *object,
|
||||
gint job);
|
||||
static inline void object_set_optional_flags (GObject *object,
|
||||
guint flags);
|
||||
static void g_object_weak_release_all (GObject *object);
|
||||
static void g_object_weak_release_all (GObject *object, gboolean release_all);
|
||||
|
||||
static void object_interface_check_properties (gpointer check_data,
|
||||
gpointer g_iface);
|
||||
@ -1828,7 +1828,7 @@ g_object_real_dispose (GObject *object)
|
||||
g_signal_handlers_destroy (object);
|
||||
|
||||
/* GWeakNotify and GClosure can call into user code */
|
||||
g_object_weak_release_all (object);
|
||||
g_object_weak_release_all (object, FALSE);
|
||||
closure_array_destroy_all (object);
|
||||
}
|
||||
|
||||
@ -3722,15 +3722,85 @@ typedef struct
|
||||
gpointer data;
|
||||
} WeakRefTuple;
|
||||
|
||||
struct _WeakRefReleaseAllState;
|
||||
|
||||
typedef struct _WeakRefReleaseAllState
|
||||
{
|
||||
guint remaining_to_notify;
|
||||
struct _WeakRefReleaseAllState *release_all_next;
|
||||
} WeakRefReleaseAllState;
|
||||
|
||||
typedef struct
|
||||
{
|
||||
guint n_weak_refs;
|
||||
guint alloc_size;
|
||||
WeakRefReleaseAllState *release_all_states;
|
||||
WeakRefTuple weak_refs[1]; /* flexible array */
|
||||
} WeakRefStack;
|
||||
|
||||
#define WEAK_REF_STACK_ALLOC_SIZE(alloc_size) (G_STRUCT_OFFSET (WeakRefStack, weak_refs) + sizeof (WeakRefTuple) * (alloc_size))
|
||||
|
||||
G_GNUC_UNUSED G_ALWAYS_INLINE static inline gboolean
|
||||
_weak_ref_release_all_state_contains (WeakRefReleaseAllState *release_all_state, WeakRefReleaseAllState *needle)
|
||||
{
|
||||
for (; release_all_state; release_all_state = release_all_state->release_all_next)
|
||||
{
|
||||
if (release_all_state == needle)
|
||||
return TRUE;
|
||||
}
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
G_ALWAYS_INLINE static inline void
|
||||
_weak_ref_stack_free (WeakRefStack *wstack)
|
||||
{
|
||||
#ifdef G_ENABLE_DEBUG
|
||||
g_assert (!wstack->release_all_states);
|
||||
#endif
|
||||
g_free (wstack);
|
||||
}
|
||||
|
||||
G_ALWAYS_INLINE static inline void
|
||||
_weak_ref_stack_update_release_all_state (WeakRefStack *wstack, guint idx)
|
||||
{
|
||||
WeakRefReleaseAllState **previous_ptr;
|
||||
WeakRefReleaseAllState *release_all_state;
|
||||
|
||||
#ifdef G_ENABLE_DEBUG
|
||||
g_assert (idx < wstack->n_weak_refs);
|
||||
#endif
|
||||
|
||||
previous_ptr = &wstack->release_all_states;
|
||||
|
||||
while (G_UNLIKELY ((release_all_state = *previous_ptr)))
|
||||
{
|
||||
if (idx >= release_all_state->remaining_to_notify)
|
||||
{
|
||||
#ifdef G_ENABLE_DEBUG
|
||||
g_assert (release_all_state->remaining_to_notify <= wstack->n_weak_refs);
|
||||
#endif
|
||||
/* We removed an index higher than the "remaining_to_notify" count. */
|
||||
goto next;
|
||||
}
|
||||
|
||||
/* Lower the "remaining_to_notify" bar of the entries we consider, as we
|
||||
* just removed an entry at index @idx (below that bar). */
|
||||
release_all_state->remaining_to_notify--;
|
||||
|
||||
if (release_all_state->remaining_to_notify > 0)
|
||||
goto next;
|
||||
|
||||
/* Remove the entry from the linked list. No need to reset
|
||||
* release_all_state->release_all_next pointer to NULL as it has no
|
||||
* purpose when not being linked. */
|
||||
*previous_ptr = release_all_state->release_all_next;
|
||||
continue;
|
||||
|
||||
next:
|
||||
previous_ptr = &release_all_state->release_all_next;
|
||||
}
|
||||
}
|
||||
|
||||
static gpointer
|
||||
g_object_weak_ref_cb (gpointer *data,
|
||||
GDestroyNotify *destroy_notify,
|
||||
@ -3745,6 +3815,7 @@ g_object_weak_ref_cb (gpointer *data,
|
||||
wstack = g_malloc (WEAK_REF_STACK_ALLOC_SIZE (1));
|
||||
wstack->alloc_size = 1;
|
||||
wstack->n_weak_refs = 1;
|
||||
wstack->release_all_states = NULL;
|
||||
i = 0;
|
||||
|
||||
*data = wstack;
|
||||
@ -3830,10 +3901,12 @@ g_object_weak_unref_cb (gpointer *data,
|
||||
wstack->weak_refs[i].data != tuple->data)
|
||||
continue;
|
||||
|
||||
_weak_ref_stack_update_release_all_state (wstack, i);
|
||||
|
||||
wstack->n_weak_refs -= 1;
|
||||
if (wstack->n_weak_refs == 0)
|
||||
{
|
||||
g_free (wstack);
|
||||
_weak_ref_stack_free (wstack);
|
||||
*data = NULL;
|
||||
}
|
||||
else
|
||||
@ -3889,13 +3962,21 @@ g_object_weak_unref (GObject *object,
|
||||
}));
|
||||
}
|
||||
|
||||
typedef struct
|
||||
{
|
||||
WeakRefReleaseAllState *const release_all_state;
|
||||
WeakRefTuple tuple;
|
||||
gboolean release_all_done;
|
||||
} WeakRefReleaseAllData;
|
||||
|
||||
static gpointer
|
||||
g_object_weak_release_all_cb (gpointer *data,
|
||||
GDestroyNotify *destroy_notify,
|
||||
gpointer user_data)
|
||||
{
|
||||
WeakRefTuple *tuple = user_data;
|
||||
WeakRefStack *wstack = *data;
|
||||
WeakRefReleaseAllData *wdata = user_data;
|
||||
WeakRefReleaseAllState *release_all_state = wdata->release_all_state;
|
||||
|
||||
if (!wstack)
|
||||
return NULL;
|
||||
@ -3904,15 +3985,72 @@ g_object_weak_release_all_cb (gpointer *data,
|
||||
g_assert (wstack->n_weak_refs > 0);
|
||||
#endif
|
||||
|
||||
if (release_all_state)
|
||||
{
|
||||
if (release_all_state->remaining_to_notify == G_MAXUINT)
|
||||
{
|
||||
if (wstack->n_weak_refs == 1u)
|
||||
{
|
||||
/* We only pop the single entry. */
|
||||
wdata->release_all_done = TRUE;
|
||||
release_all_state = NULL;
|
||||
}
|
||||
else
|
||||
{
|
||||
release_all_state->remaining_to_notify = wstack->n_weak_refs;
|
||||
|
||||
/* Prepend to linked list. */
|
||||
release_all_state->release_all_next = wstack->release_all_states;
|
||||
wstack->release_all_states = release_all_state;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
if (release_all_state->remaining_to_notify == 0u)
|
||||
{
|
||||
#ifdef G_ENABLE_DEBUG
|
||||
g_assert (!_weak_ref_release_all_state_contains (wstack->release_all_states, release_all_state));
|
||||
#endif
|
||||
return NULL;
|
||||
}
|
||||
#ifdef G_ENABLE_DEBUG
|
||||
g_assert (release_all_state->remaining_to_notify <= wstack->n_weak_refs);
|
||||
g_assert (_weak_ref_release_all_state_contains (wstack->release_all_states, release_all_state));
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
_weak_ref_stack_update_release_all_state (wstack, 0);
|
||||
|
||||
if (release_all_state && release_all_state->remaining_to_notify == 0)
|
||||
wdata->release_all_done = TRUE;
|
||||
|
||||
wstack->n_weak_refs--;
|
||||
|
||||
/* Emit the notifications in FIFO order. */
|
||||
*tuple = wstack->weak_refs[0];
|
||||
wdata->tuple = wstack->weak_refs[0];
|
||||
|
||||
if (wstack->n_weak_refs == 0)
|
||||
{
|
||||
g_free (wstack);
|
||||
_weak_ref_stack_free (wstack);
|
||||
*data = NULL;
|
||||
|
||||
/* Also set release_all_done.
|
||||
*
|
||||
* If g_object_weak_release_all() was called during dispose (with
|
||||
* release_all FALSE), we anyway have an upper limit of how many
|
||||
* notifications we want to pop. We only pop the notifications that were
|
||||
* registered when the loop initially starts. In that case, we surely
|
||||
* don't want the caller to call back.
|
||||
*
|
||||
* g_object_weak_release_all() is also being called before finalize. At
|
||||
* that point, the ref count is already at zero, and g_object_weak_ref()
|
||||
* asserts against being called. So nobody can register a new weak ref
|
||||
* anymore.
|
||||
*
|
||||
* In both cases, we don't require the calling loop to call back. This
|
||||
* saves an additional GData lookup. */
|
||||
wdata->release_all_done = TRUE;
|
||||
}
|
||||
else
|
||||
{
|
||||
@ -3920,34 +4058,37 @@ g_object_weak_release_all_cb (gpointer *data,
|
||||
&wstack->weak_refs[1],
|
||||
sizeof (wstack->weak_refs[0]) * wstack->n_weak_refs);
|
||||
|
||||
/* Don't bother shrinking the buffer. The caller will loop until all
|
||||
* elements are removed. */
|
||||
/* Don't bother to shrink the buffer. Most likely the object gets
|
||||
* destroyed soon after. */
|
||||
}
|
||||
|
||||
return tuple;
|
||||
return wdata;
|
||||
}
|
||||
|
||||
static void
|
||||
g_object_weak_release_all (GObject *object)
|
||||
g_object_weak_release_all (GObject *object, gboolean release_all)
|
||||
{
|
||||
WeakRefTuple tuple;
|
||||
WeakRefReleaseAllState release_all_state = {
|
||||
.remaining_to_notify = G_MAXUINT,
|
||||
};
|
||||
WeakRefReleaseAllData wdata = {
|
||||
.release_all_state = release_all ? NULL : &release_all_state,
|
||||
.release_all_done = FALSE,
|
||||
};
|
||||
|
||||
/* We notify weak references in a loop. As this emits external callbacks, a
|
||||
* callee could register another weak reference, which the loop would notify
|
||||
* right away. This means, registering new weak references during dispose
|
||||
* does not work well, which you might want to do when resurrecting the
|
||||
* object under destruction.
|
||||
*
|
||||
* This is an intentional choice. It would be complicated to keep track of
|
||||
* the tuples that were present when the loop starts, and only notify those.
|
||||
*
|
||||
* You are advised to not register new weak references while handling a weak
|
||||
* notification. */
|
||||
while (_g_datalist_id_update_atomic (&object->qdata,
|
||||
quark_weak_notifies,
|
||||
g_object_weak_release_all_cb,
|
||||
&tuple))
|
||||
tuple.notify (tuple.data, object);
|
||||
while (TRUE)
|
||||
{
|
||||
if (!_g_datalist_id_update_atomic (&object->qdata,
|
||||
quark_weak_notifies,
|
||||
g_object_weak_release_all_cb,
|
||||
&wdata))
|
||||
break;
|
||||
|
||||
wdata.tuple.notify (wdata.tuple.data, object);
|
||||
|
||||
if (wdata.release_all_done)
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -4691,7 +4832,7 @@ retry_decrement:
|
||||
|
||||
closure_array_destroy_all (object);
|
||||
g_signal_handlers_destroy (object);
|
||||
g_object_weak_release_all (object);
|
||||
g_object_weak_release_all (object, TRUE);
|
||||
|
||||
TRACE (GOBJECT_OBJECT_FINALIZE (object, G_TYPE_FROM_INSTANCE (object)));
|
||||
G_OBJECT_GET_CLASS (object)->finalize (object);
|
||||
|
||||
@ -285,19 +285,80 @@ test_references (void)
|
||||
/*****************************************************************************/
|
||||
|
||||
static guint weak_ref4_notified;
|
||||
static guint weak_ref4_finalizing_notified;
|
||||
static guint weak_ref4_finalizing_notified_skipped;
|
||||
static gint weak_ref4_finalizing_resurrected = -1;
|
||||
static guint weak_ref4_indexes_n;
|
||||
static const guint *weak_ref4_indexes;
|
||||
|
||||
static void
|
||||
weak_ref4_finalizing (gpointer data,
|
||||
GObject *object)
|
||||
{
|
||||
static gint last = -1;
|
||||
gint idx;
|
||||
|
||||
g_assert_true (weak_ref4_finalizing_resurrected == FALSE || weak_ref4_finalizing_resurrected == 4);
|
||||
|
||||
idx = (gint) GPOINTER_TO_UINT (data);
|
||||
g_assert_cmpint (last, <, idx);
|
||||
last = idx;
|
||||
|
||||
weak_ref4_finalizing_notified++;
|
||||
}
|
||||
|
||||
static void
|
||||
weak_ref4 (gpointer data,
|
||||
GObject *object)
|
||||
{
|
||||
static gint last = -1;
|
||||
gint idx;
|
||||
guint idx;
|
||||
|
||||
idx = (gint) GPOINTER_TO_UINT (data);
|
||||
g_assert_cmpint (last, <, idx);
|
||||
last = idx;
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified, ==, 0);
|
||||
|
||||
idx = GPOINTER_TO_UINT (data);
|
||||
g_assert_cmpint (last, <, (gint) idx);
|
||||
last = (gint) idx;
|
||||
|
||||
weak_ref4_notified++;
|
||||
|
||||
if (weak_ref4_finalizing_resurrected == -1)
|
||||
{
|
||||
if (g_random_boolean ())
|
||||
{
|
||||
/* Resurrect the object. */
|
||||
weak_ref4_finalizing_resurrected = TRUE;
|
||||
g_object_ref (object);
|
||||
}
|
||||
else
|
||||
weak_ref4_finalizing_resurrected = FALSE;
|
||||
}
|
||||
|
||||
g_object_weak_ref (object, weak_ref4_finalizing, GUINT_TO_POINTER (idx));
|
||||
|
||||
if (weak_ref4_indexes_n > 0 && (g_random_int () % 4 == 0))
|
||||
{
|
||||
|
||||
while (weak_ref4_indexes_n > 0 && weak_ref4_indexes[weak_ref4_indexes_n - 1] <= idx)
|
||||
{
|
||||
/* already invoked. Skip. */
|
||||
weak_ref4_indexes_n--;
|
||||
}
|
||||
|
||||
/* From the callback, we unregister another callback that we know is
|
||||
* still registered. */
|
||||
if (weak_ref4_indexes_n > 0)
|
||||
{
|
||||
guint new_idx = weak_ref4_indexes[weak_ref4_indexes_n - 1];
|
||||
|
||||
weak_ref4_indexes_n--;
|
||||
g_object_weak_unref (object, weak_ref4, GUINT_TO_POINTER (new_idx));
|
||||
|
||||
/* Behave as if we got this callback invoked still, so that the remainder matches up. */
|
||||
weak_ref4_finalizing_notified_skipped++;
|
||||
weak_ref4_notified++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
@ -330,8 +391,29 @@ test_references_many (void)
|
||||
m = g_random_int () % (n + 1u);
|
||||
for (i = 0; i < m; i++)
|
||||
g_object_weak_unref (object, weak_ref4, GUINT_TO_POINTER (indexes[i]));
|
||||
weak_ref4_indexes_n = n - m;
|
||||
weak_ref4_indexes = &indexes[m];
|
||||
g_object_unref (object);
|
||||
g_assert_cmpint (weak_ref4_notified, ==, n - m);
|
||||
if (n - m == 0)
|
||||
{
|
||||
g_assert_cmpint (weak_ref4_finalizing_resurrected, ==, -1);
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified, ==, 0);
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified_skipped, ==, 0);
|
||||
}
|
||||
else if (weak_ref4_finalizing_resurrected == TRUE)
|
||||
{
|
||||
weak_ref4_finalizing_resurrected = 4;
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified, ==, 0);
|
||||
g_object_unref (object);
|
||||
g_assert_cmpint (weak_ref4_notified, ==, n - m);
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified + weak_ref4_finalizing_notified_skipped, ==, n - m);
|
||||
}
|
||||
else
|
||||
{
|
||||
g_assert_cmpint (weak_ref4_finalizing_resurrected, ==, FALSE);
|
||||
g_assert_cmpint (weak_ref4_finalizing_notified + weak_ref4_finalizing_notified_skipped, ==, n - m);
|
||||
}
|
||||
g_free (indexes);
|
||||
}
|
||||
|
||||
@ -368,6 +450,105 @@ test_references_two (void)
|
||||
|
||||
/*****************************************************************************/
|
||||
|
||||
#define RUN_DISPOSE_N_THREADS 5
|
||||
|
||||
GThread *run_dispose_threads[RUN_DISPOSE_N_THREADS];
|
||||
GObject *run_dispose_obj;
|
||||
gint run_dispose_thread_ready_count;
|
||||
const guint RUN_DISPOSE_N_ITERATIONS = 5000;
|
||||
|
||||
gint run_dispose_weak_notify_pending;
|
||||
gint run_dispose_weak_notify_pending_nested;
|
||||
|
||||
static void
|
||||
_run_dispose_weak_notify_nested (gpointer data,
|
||||
GObject *where_the_object_was)
|
||||
{
|
||||
g_assert_true (run_dispose_obj == where_the_object_was);
|
||||
g_assert_cmpint (g_atomic_int_add (&run_dispose_weak_notify_pending_nested, -1), >, 0);
|
||||
}
|
||||
|
||||
static void
|
||||
_run_dispose_weak_notify (gpointer data,
|
||||
GObject *where_the_object_was)
|
||||
{
|
||||
g_assert_true (run_dispose_obj == where_the_object_was);
|
||||
g_assert_cmpint (g_atomic_int_add (&run_dispose_weak_notify_pending, -1), >, 0);
|
||||
|
||||
if (g_random_int () % 10 == 0)
|
||||
{
|
||||
g_object_run_dispose (run_dispose_obj);
|
||||
}
|
||||
|
||||
if (g_random_int () % 10 == 0)
|
||||
{
|
||||
g_atomic_int_inc (&run_dispose_weak_notify_pending_nested);
|
||||
g_object_weak_ref (run_dispose_obj, _run_dispose_weak_notify_nested, NULL);
|
||||
}
|
||||
}
|
||||
|
||||
static gpointer
|
||||
_run_dispose_thread_fcn (gpointer thread_data)
|
||||
{
|
||||
guint i;
|
||||
|
||||
g_atomic_int_inc (&run_dispose_thread_ready_count);
|
||||
|
||||
while (g_atomic_int_get (&run_dispose_thread_ready_count) != RUN_DISPOSE_N_THREADS)
|
||||
g_usleep (10);
|
||||
|
||||
for (i = 0; i < RUN_DISPOSE_N_ITERATIONS; i++)
|
||||
{
|
||||
g_atomic_int_inc (&run_dispose_weak_notify_pending);
|
||||
g_object_weak_ref (run_dispose_obj, _run_dispose_weak_notify, NULL);
|
||||
if (g_random_int () % 10 == 0)
|
||||
g_object_run_dispose (run_dispose_obj);
|
||||
}
|
||||
|
||||
g_object_run_dispose (run_dispose_obj);
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
static void
|
||||
test_references_run_dispose (void)
|
||||
{
|
||||
GQuark quark_weak_notifies = g_quark_from_static_string ("GObject-weak-notifies");
|
||||
guint i;
|
||||
|
||||
run_dispose_obj = g_object_new (G_TYPE_OBJECT, NULL);
|
||||
|
||||
for (i = 0; i < RUN_DISPOSE_N_THREADS; i++)
|
||||
{
|
||||
run_dispose_threads[i] = g_thread_new ("run-dispose", _run_dispose_thread_fcn, GINT_TO_POINTER (i));
|
||||
}
|
||||
for (i = 0; i < RUN_DISPOSE_N_THREADS; i++)
|
||||
{
|
||||
g_thread_join (run_dispose_threads[i]);
|
||||
}
|
||||
|
||||
g_assert_cmpint (g_atomic_int_get (&run_dispose_weak_notify_pending), ==, 0);
|
||||
|
||||
if (g_atomic_int_get (&run_dispose_weak_notify_pending_nested) == 0)
|
||||
{
|
||||
g_assert_null (g_object_get_qdata (run_dispose_obj, quark_weak_notifies));
|
||||
}
|
||||
else
|
||||
{
|
||||
g_assert_nonnull (g_object_get_qdata (run_dispose_obj, quark_weak_notifies));
|
||||
}
|
||||
|
||||
g_object_run_dispose (run_dispose_obj);
|
||||
|
||||
g_assert_cmpint (g_atomic_int_get (&run_dispose_weak_notify_pending), ==, 0);
|
||||
g_assert_cmpint (g_atomic_int_get (&run_dispose_weak_notify_pending_nested), ==, 0);
|
||||
g_assert_null (g_object_get_qdata (run_dispose_obj, quark_weak_notifies));
|
||||
|
||||
g_clear_object (&run_dispose_obj);
|
||||
}
|
||||
|
||||
/*****************************************************************************/
|
||||
|
||||
int
|
||||
main (int argc,
|
||||
char *argv[])
|
||||
@ -381,6 +562,7 @@ main (int argc,
|
||||
g_test_add_func ("/gobject/references", test_references);
|
||||
g_test_add_func ("/gobject/references-many", test_references_many);
|
||||
g_test_add_func ("/gobject/references_two", test_references_two);
|
||||
g_test_add_func ("/gobject/references_run_dispose", test_references_run_dispose);
|
||||
|
||||
return g_test_run ();
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user