Previously:
- if the object is currently not frozen, we called
g_object_notify_queue_freeze() once. Afterwards dispatch the event
directly. This is probably the common case, and requires one
notify_lock lock.
- if the object is currently frozen, we call
g_object_notify_queue_freeze(), g_object_notify_queue_add().
g_object_notify_queue_thaw().
This required taking the notify_lock three times.
- if the object is currently not frozen and in_init, then we called
g_object_notify_queue_freeze(), g_object_notify_queue_freeze(),
g_object_notify_queue_add(). This also required to take
the lock three times. There is another thaw at the end of
object initialization.
That was because we first call g_object_notify_queue_freeze() to see
whether we are frozen. And depending on that, queue the event (and thaw
again).
Instead, g_object_notify_queue_add() can do the check and queueing in
one step. There is no need to call a freeze() to (conditionally) enqueue
a notification. Now only one lock is taken in all cases.
Also, g_object_notify_queue_freeze() and g_object_notify_queue_thaw()
both call g_datalist_id_get_data() (which also take a bit lock). As the
thaw is no longer necessary, the second lock is also saved.
Before dispatching signals (and calling out to user code), we want to
take a reference and ensure that the object stays alive.
However, a thaw may not decrease the freeze_count to zero, or there may
be no properties to notify. Avoid taking a reference in those cases.
This was done since the beginning (commit e773d7dba6 ('fixed dealing
with collection/lcopy of NULL values.'). But it's not clear, why we
would need to take a reference on the calling object.
Freeze does not emit any signals/callbacks and does not call back to the
user. It just sets up some internal state.
This doesn't require to take a reference. The caller must hold a valid
reference to being with, but if that's given, there is no need to
acquire another reference.
g_atomic_int_get() returns a signed int. While we don't expect this to be ever
negative, a negative value would also indicate a bug. Adjust the check to assert
against negative ref-count too.
Right now, we're assuming that GObjectClass will be initialised first
and under a lock, but that's not always the case: when traversing a list
of type, the first one might be a GTypeInterface, and if we initialise
an interface that installs a property, the whole thing comes crashing
down because the global GParamSpecPool is not initialised.
Instead of taking a lock everywhere, we can use an atomic compare and
swap; the first thread that installs a property wins the race, as any
other access to the GParamSpecPool is performed under a lock.
Nicks and blurbs don't have any practical use for gio/gobject libraries.
Leaving tests untouched since this features is still used by other libraries.
Closes#2991
Round-tripping pointers via gsize is not guaranteed to work (the C standard
only requires this for (u)inptr_t) and in fact breaks on CHERI-enabled
systems such as Arm Morello where pointers are 128-bits but size_t is 64.
This means the current casts would strip the high bits of the pointer and
return a non-dereferenceable value. Fix this by casting the operand that
holds the pointer to guintptr instead of gsize.
Helps: https://gitlab.gnome.org/GNOME/glib/-/issues/2842
This should not result in any functional changes, but will eventually
allow glib to be functional on CHERI-enabled systems such as Morello.
Helps: https://gitlab.gnome.org/GNOME/glib/-/issues/2842
When building for CHERI with additional warning flags, implicitly
converting uintptr_t to an integer type that can't store a pointer
results in a compiler warnings. Silence two of these by adding
explicit casts.
This patch is based upon Garrett Regier's work from 2015 to provide
some reliability and predictability to how disposal handles weak
reference state.
A primary problem is that GWeakRef and GWeakNotify state is shared and
therefore you cannot rely on GWeakRef status due to a GWeakNotify
calling into second-degree code.
It's important to ensure that both weak pointer locations and GWeakRef
will do the proper thing before user callbacks are executed during
disposal. Otherwise, user callbacks cannot rely on the status of their
weak pointers. That would be mostly okay but becomes an issue when
second degree objects are then disposed before any notification of
dependent object disposal.
Consider objects A and B.
`A` contains a reference to `B` and `B` contains a `GWeakRef` to `A`.
When `A` is disposed, `B` may be disposed as a consequence but has not
yet been notified that `A` has been disposed. It's `GWeakRef` may also
cause liveness issues if `GWeakNotify` on `A` result in tertiary code
running which wants to interact with `B`.
This example is analagous to how `GtkTextView` and `GtkTextBuffer` work
in text editing applications.
To provide application and libraries the ability to handle this using
already existing API, `GWeakRef` is separated into it's own GData quark
so that weak locations and `GWeakRef` are cleared before user code is
executed as a consequence of `GData` cleanup.
# Conflicts:
# gobject/tests/signals.c
GTK lost it's '+' suffix back in 2019, according to
<https://mail.gnome.org/archives/gtk-devel-list/2019-February/msg00000.html>
This commit can be re-generated with:
git grep -l GTK+ \
| grep -v -e ^NEWS -e ^glib/tests/collate.c \
| xargs sed -i 's/GTK+/GTK/g'
Most of the changes are in comments and documentation.
Instead of a plain reference count check failure that is really hard to
understand, let's be explicit, and warn that manipulating an object's
notification queue during its finalization is not allowed.
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.
We were reading if an object has toggle references even if this was not
really relevant for the current object state, as we only need to notify
when going from 2 to 1 references, so first ensure that this is the case
and then check if we have toggle references enabled in the object.
This is a micro-optimization, for the way flags are defined, but still
an operation we can avoid in most cases.
Even though the check is likely to be relevant if the object is finalized,
it may still give some indication if called while an instance has just lost
the last reference.
So use `g_return_if_fail` for consistency with the rest of the code.
In case g_atomic_int_compare_and_exchange() check fails we ended up doing
another atomic get to figure out what it was the old reference count,
however, we can avoid this by using the full version of the function that
returns the value before the exchange happened as an out value.
If a deprecated property only gets set because it is G_PARAM_CONSTRUCT
or G_PARAM_CONSTRUCT_ONLY, then there is nothing for the library user
to fix, and we should not emit a deprecation warning.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2748
Signed-off-by: Simon McVittie <smcv@collabora.com>
All of these warnings indicate programmer error, so critical is most
appropriate here.
Exceptions: deprecation warnings are just warnings. Also, warnings that
are worded with uncertainty can remain warnings rather than criticals.
When I optimized GObject to skip property notification
in some cases, I looked for whether the class has a
custom notify vfunc. I overlooked that that
dispatch_properties_changed can also be customized,
and if it is, we better not skip change notification.
This showed up as breakage in the adjustment tests
in the GTK testsuite.
When weak references are being cleaned up, it is possible for the `qdata` for
both `quark_weak_locations` and `quark_weak_refs` to have been deallocated,
so that `g_datalist_id_get_data()` returns `NULL` for both. This happens
when `g_object_run_dispose()` is called for the target of a `GBinding`,
and is not an error.
See https://gitlab.gnome.org/GNOME/glib/-/issues/2676
This makes calls to g_signal_connect_data() and g_signal_connect_object()
with default flags more self-documenting.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This is a partial revert of commit fa8c7c0da using the approach
suggested (and tested) by Kjell Ahlstedt.
It is intended to be temporary pending a proper dig into what’s causing
the regression, just so we can get the 2.73.1 release out.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2672
Coverity notices the `g_object_unref()` call in `g_object_notify()`, but
not the paired `g_object_ref()` call. It therefore incorrectly assumes
that every call to `g_object_notify()` frees the object. This causes a
lot (hundreds) of false positive reports about double-frees or
use-after-frees.
I can’t find a way to fix this using a model file, so the other options
are:
* Manually mark every report as a false positive and keep updating them
as the code changes over time. This would take a lot of maintainer
effort.
* Comment out the `g_object_ref()`/`g_object_unref()` calls when
running static analysis (but not in a normal production build). This
is ugly, but cheap and shouldn’t impact maintainability much.
So this commit implements option 2.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This prevents `-Wunused-function` warnings on platforms which don’t have
`HAVE_OPTIONAL_FLAGS` defined.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
g_object_new_with_custom_constructor needs to handle
freezing notifications in the same way as
g_object_new_internal.
Fixing a bug pointed out by Christian Hergert.
