GSList doesn't seem the best choice here. It's benefits are that it's
relatively convenient to use (albeit not very efficient) and that an
empty list requires only the pointer to the list's head.
But for non-empty list, we need to allocate GSList elements. We can do
better, by writing more code.
I think it's worth optimizing GObject, at the expense of a bit(?) more
complicated code. The complicated code is still entirely self-contained,
so unless you review WeakRefData usage, it doesn't need to bother you.
Note that this can be easily measure to be a bit faster. But I think the
more important part is to safe some allocations. Often objects are
long-lived, and the GWeakRef will be tracked for a long time. It is
interesting, to optimize the memory usage of that.
- if the list only contains one weak reference, it's interned/embedded in
WeakRefData.list.one. Otherwise, an array is allocated and tracked
at WeakRefData.list.many.
- when the buffer grows, we double the size. When the buffer shrinks,
we reallocate to 50% when 75% are empty. When the buffer shrinks to
length 1, we free it (so that "list.one" is always used with a length
of 1).
That means, at worst case we waste 75% of the allocated buffer,
which is a choice in the hope that future weak references will be
registered, and that this is a suitable strategy.
- on architectures like x86_68, does this not increase the size of
WeakRefData.
Also, the number of weak-refs is now limited to 65535, and now an
assertion fails when you try to register more than that. But note that
the internal tracking just uses a linear search, so you really don't
want to register thousands of weak references on an object. If you do
that, the current implementation is not suitable anyway and you must
rethink your approach. Nor does it make sense to optimize the
implementation for such a use case. Instead, the implementation is
optimized for a few (one!) weak reference per object.
The implementation of GWeakRef tracks weak references in a way, that
requires linear search. That is probably best, for an expected low
number of entries (e.g. compared to the overhead of having a hash
table). However, it means, if you create thousands of weak references,
performance start to degrade.
Add a test that creates 64k weak references. Just to see how it goes.
In general, we must not call out to external, unknown code while holding
a lock. That is prone to dead lock.
g_object_ref() can emit a toggle notification. In g_weak_ref_set(), we
must not do that while holding the lock.
GWeakRef calls g_object_ref() while holding a read lock.
g_object_ref() can emit a toggle notification.
If you then inside the toggle notification setup a GWeakRef
(to the same or another object), the code will try to get
a write lock. Deadlock will happen.
Add a test to "show" that (the breaking part is commented out).
Will be fixed next.
In g_object_unref(), we call _object_unref_clear_weak_locations() before
and after dispose() already. At those places it is necessary to do.
Calling it a third time during g_object_real_dispose() seems not useful,
has unnecessary overhead and actually undesirable.
In particular, because g_object_real_dispose() is the implementation for
the virtual function GObject.dispose(). For subclasses that override dispose(),
it's not well defined at which point they should chain up the parent
implementation (for dispose(), I'd argue that usually they chain up at
the end of their own code). If they chain up at the end, this has no
effect.
This only really matters if you try to register GWeakRef during dipose
and/or resurrect the object.
static void dispose(GObject *object)
{
g_weak_ref_set(&global_weak_ref, object);
global_ref = g_object_ref(object);
G_OBJECT_CLASS (parent_class)->dispose (object);
}
the object was resurrected, but g_object_real_dispose() would clear the
weak ref. That is not desirable, nor does it make sense.
Instead, the virtual function dispose() is called from two places, from
g_object_unref() and g_object_run_dispose(). In both cases, it is
ensured that weak locations are cleared *after* dispatching the virtual
function. Don't do it somewhere in the middle from
g_object_real_dispose().
The previous g_object_unref() was racy. There were three places where we
decremented the ref count, but still accessed the object afterwards
(while assuming that somebody else might still hold a reference). For
example:
if (!g_atomic_int_compare_and_exchange_full ((int *) &object->ref_count,
old_ref, old_ref - 1,
&old_ref))
continue;
TRACE (GOBJECT_OBJECT_UNREF (object, G_TYPE_FROM_INSTANCE (object), old_ref));
/* if we went from 2->1 we need to notify toggle refs if any */
if (old_ref == 2 && OBJECT_HAS_TOGGLE_REF (object))
{
/* The last ref being held in this case is owned by the toggle_ref */
toggle_refs_notify (object, TRUE);
}
After we decrement the reference count (and gave up our reference), we
are only allowed to access object if we know we have the only possible
reference to it. In particular, if old_ref is larger than 1, then
somebody else holds references and races against destroying object.
The object might be a dangling pointer already.
This is slightly complicated due to toggle references and clearing of
weak-locations.
For toggle references, we must take a lock on the mutex. Luckily, that
is only necessary, when the current reference count is exactly 2.
Note that we emit the TRACE() after the ref count was already decreased.
If another thread unrefs the object, inside the TRACE() we might have a
dangling pointer. That would only be fixable, by emitting the TRACE()
before the actual unref (which has its own problems). This problem
already existed previously.
The change to the test is necessary and correct. Before this patch,
g_object_unref() would call dispose() and decrement the reference count
right after.
In the test case at gobject/tests/reference.c:1108, the reference count
after dispose and decrement is 1. Then it thaws the queue notification,
which emits a property changed signal. The test then proceeds to
reference the object again and notifying the toggle reference.
Previously, the toggle reference was notified 3 times.
After this change, the property changed signal is emitted before
decreasing the reference count. Taking a reference then does not cause
an additional toggle on+off, so in total only one toggle happens.
That accounts for the change in the test. The new behavior is
correct.
When an object is revitalized and a notify callbacks increased the reference
counter of the object, we are calling the toggle notifier twice, while it
should only happen if also the actual reference count value is 1 (after
having been decremented from 2).
If an object gets revitalized during the dispose vfunc, we need to call
toggle refs notifiers only if we had 2 references and if the object has
the toggle references enabled.
This may change in case an object notifier handler changes this status,
so do this check only after we've called the notifiers so that in case
toggle notifications are enabled afterwards we still call the handlers.
This was previously exercised (probably by mistake) in
gobject/tests/type.c, but without making any assertions about what
happened, and the test would fail under G_ENABLE_DIAGNOSTIC if GLib was
compiled with debug enabled.
Signed-off-by: Simon McVittie <smcv@collabora.com>
As per the previous change, an object that had weak locations set may
need to lock again the weak locations mutex during qdata cleanup, but
we can avoid this when we know we're removing the last location, by
removing the qdata entry and freeing the data.
In case a new location is needed for the same object, new data will be
added.
However, by doing this the weak locations during dispose may be
invalidated once the weak locations lock is passed, so check again if
this is the case while removing them.
It can happen that a GWeakRef is added to an object while it's disposing
(or even during finalizing) and this may happen in a thread that (weak)
references an object while the disposal isn't completed yet or when
using toggle references and switching to GWeakRef on notification (as
the API suggests).
In such scenario the weak locations are not cleaned up when the object
is finalized, and will point to a free'd area.
So, during finalization and when we're sure that the object will be
destroyed for sure, check again if there are new weak locations and
unset them if any as part of the qdata destruction.
Do this adding a new utility function so that we can avoid duplicating
code to free the weak locations.
Added various tests simulating this case.
Fixes: #2390
This works in the same way as g_variant_take_ref(), and for the same
reason.
Updated and Rebased by Nitin Wartkar <nitinwartkar58@gmail.com>
Closes#1112
When calling `g_set_object()` for a type derived from `GObject`, GCC 9.2
was giving the following strict aliasing warning:
```
../../source/malcontent/libmalcontent-ui/user-controls.c:1001:21: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
1001 | if (g_set_object (&self->user, user))
/opt/gnome/install/include/glib-2.0/gobject/gobject.h:744:33: note: in definition of macro ‘g_set_object’
744 | (g_set_object) ((GObject **) (object_ptr), (GObject *) (new_object)) \
| ^~~~~~~~~~
```
This was due to the `(GObject **)` cast.
Pass the pointer through a union to squash this warning. We already do
some size and type checks of the dereferenced type, which should catch
casual errors. The `g_object_ref()` and `g_object_unref()` calls which
subsequently happen inside the `g_set_object()` function also do some
dynamic type checks.
Add a test.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The implementation is silently discarding this anyway, and
g_object_unref() is using atomic operations. So this should be safe.
Having this here triggers -Wdiscarded-qualifiers when g_clear_pointer()
is fixed to use __typeof__().
The implementation is silently discarding this anyway, and
g_object_unref() is using atomic operations. So this should be safe.
Having this here triggers -Wdiscarded-qualifiers when g_clear_pointer()
is fixed to use __typeof__().
Switch the check which tests whether the object has been finalised from
being a use-after-free, to using a weak pointer which is nullified on
finalisation.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Weak-pointers are currently lacking g_set_object() & g_clear_object()
helpers equivalent. New functions (and macros, both are provided) are
convenient in many case, especially for the property's notify-on-set
pattern:
if (g_set_weak_pointer (...))
g_object_notify (...)
Inspired by Christian Hergert's original implementation for
gnome-builder.
https://bugzilla.gnome.org/show_bug.cgi?id=749527
Along the same lines as g_clear_object(), g_set_object() is a
convenience function to update a GObject pointer, handling reference
counting transparently and correctly.
Specifically, it handles the case where a pointer is set to its current
value. If handled naïvely, that could result in the object instance
being finalised. In the following code, that happens when
(my_obj == new_value) and the object has a single reference:
g_clear_object (&my_obj);
my_obj = g_object_ref (new_value);
It also simplifies boilerplate code such as set_property()
implementations, which are otherwise long and boring.
Test cases included.
https://bugzilla.gnome.org/show_bug.cgi?id=741589
Very many testcases, some GLib tools (resource compiler, etc) and
GApplication were calling g_type_init().
Remove those uses, as they are no longer required.
https://bugzilla.gnome.org/show_bug.cgi?id=686161
