When an object with toggle reference is notifying a change we just
assume that this is true because of previous checks.
However, while locking, another thread may have removed the toggle
reference causing the waiting thread to abort (as no handler is set at
that point).
To avoid this, once we've got the toggle references mutex lock, check
again if the object has toggle reference, and if it's not the case
anymore just ignore the request.
Add a test that triggers this, it's not 100% happening because this is
of course timing related, but this is very close to the truth.
Fixes: #2394
The previous wording was not clear about what happens if a new weak ref
is taken during disposal (shortly after resurrecting the object with a
new strong ref, otherwise taking the weak ref is invalid).
See: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2064/diffs#note_1270092
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2390
No need to call memset in the loop, we can just
initialize all the values in one go.
GtkBuilder is now using g_object_setv, so this
may improve application start times a bit.
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
GTK currently checks if a GtkWidget is finalized while still using a
floating reference—i.e. a widget was disposed without any parent
container owning it.
This warning can be useful to identify and trace ownership transfer
issues in libraries using initially unowned floating object types.
To avoid introducing constraints ex post, we can gate this check behind
both the G_ENABLE_DEBUG compile time flag for GLib, and behind the
G_ENABLE_DIAGNOSTIC environment variable run time check.
Fixes: #2489
When rendering the contents of the GLib documentation stored inside the
introspection data, a common behaviour is to take the first paragraph as
a summary of the symbol being documented.
The documentation is assumed to be in Markdown format, which means:
- paragraphs must be separated by newlines
- lines that have an indentation of four or more spaces are considered
code blocks
- lines that start with a `#` are considered titles
This means we need to slightly tweak the documentation in our sources to
ensure that it can be rendered appropriately by tools that are not
gtk-doc.
See issue: #2365
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
gjs has some situations where it's not always aware of the @data that
was passed into g_object_add_toggle_ref, so allow passing %NULL to
just match on @notify.
Rebased and updated by Nitin Wartkar
Closes#817
This especially has the effect that any GWeakRefs to the object will not
necessarily be set to NULL yet if called as part of
g_object_run_dispose() and not as part of g_object_unref().
gobject/gobject.c: In function ‘g_object_new_internal’:
gobject/gobject.c:1962:25: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1962 | for (j = 0; j < n_params; j++)
| ^
gobject/gobject.c:1989:21: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1989 | for (i = 0; i < n_params; i++)
| ^
gobject/gobject.c: In function ‘g_object_new_with_custom_constructor’:
gobject/gobject.c:1836:21: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1836 | for (j = 0; j < n_params; j++)
| ^
gobject/gobject.c:1914:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1914 | for (i = 0; i < n_params; i++)
| ^
gobject/gobject.c: In function ‘g_object_class_install_properties’:
gobject/gobject.c:766:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
766 | for (i = 1; i < n_pspecs; i++)
| ^
These variables were already (correctly) accessed atomically. The
`volatile` qualifier doesn’t help with that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
The previous code consumed a larger additional amount of stack space.
That is because it would allocate the temporary buffer for GValues on
the stack with "g_newa (GValue, 1)" and thus the required stack
space grew with the number of arguments. Granted, this is already
a variadic C function, so the caller already placed that many elements
on the stack. For example, on the stack there are the property names
and the pointers to the arguments, which should amount to roughly
O(n_args * 16) (on 64 bit, with pointers being 8 bytes large).
That is not bad, because it means in the previous version the stack space
would grow linear with the already used stack space. However, a GValue is
an additional 24 bytes (on 64 bit), which probably more than doubles the
required stack space. Let's avoid that, by allocating the temporary list
on the heap after a certain threshold. This probably more than doubles the
number of possible arguments before the stack overflows.
Also, previously the heap allocated "params" array only grew one element
per iteration. Of course, it is likely that libc anyway reallocates
the buffers by growing the space exponentially. So realloc(ptr, 1)
probably does not O() scale worse than doubling the buffer sizes ourselves.
However, it seems clearer to keep track of the allocated sizes ourself, and
only call realloc() when we determine that we are out of space.
Especially because we need to update the value pointers on reallocation.
Note that we now require a heap allocation both for the "params" and the
"values" list. Theoretically that could be combined by using one buffer
for both. But that would make the code more complicated.
Now we pre-allocate buffers for 16 elements on the stack. That
is (16 * (16 + 24) bytes (or 640 bytes) on the stack. I think that
is still acceptable.
Two out of three callers pass the count argument from a variable
of type guint. And the third is currently an always positive gint.
We should use the correct integer type that matches the type as it
used otherwise.
Rather than using a mixture of ‘instantiable’ and ‘instantiatable’
everywhere, standardise on the term which is already in the public API.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This was mostly machine generated with the following command:
```
codespell \
--builtin clear,rare,usage \
--skip './po/*' --skip './.git/*' --skip './NEWS*' \
--write-changes .
```
using the latest git version of `codespell` as per [these
instructions](https://github.com/codespell-project/codespell#user-content-updating).
Then I manually checked each change using `git add -p`, made a few
manual fixups and dropped a load of incorrect changes.
There are still some outdated or loaded terms used in GLib, mostly to do
with git branch terminology. They will need to be changed later as part
of a wider migration of git terminology.
If I’ve missed anything, please file an issue!
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The various `g_strdup_printf()` returns values in the implementations of GValue
lcopy_func are runtime checks which could be disabled if one wants and therefore
should be handled as such with g_return_val_if_fail()
Rename the variables involved so that people get a slightly more
obvious critical warning when they try to ref an object which has
already been finalised.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
As with `g_variant_new()` (or any varargs function which takes integer
literals of differing widths), callers need to be careful to ensure
their integer literals have the right width.
Tweak the documentation for `g_object_new()`, `g_object_set()` and
`g_object_get()` to clarify this. The documentation for `g_object_get()`
shows that it is not subject to the same caveats, since it operates on
pointers.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #833
This uses a 32bit hole in the GObject structure on 64bit arches
as a flag field which can be optionally used for some preformance hints.
Currently there is a flag that gets set any time you connect to a signal
on a GObject which is used as early bailout for signal emissions, and using
the flags field instead of a user-data for checking if a GObject is
under construction.
"Uninitialized value" is partially correct, since it has not been
initialized with a type, but it's more precise to say
"zero-initialized value". It is still a programming error to pass a
pointer to uninitialized memory with arbitrary contents as the value.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Using the generic marshaller has drawbacks beyond performance. One such
drawback is that it breaks the stack unwinding from the Linux kernel due
to having unsufficient data to walk past ffi_call_unixt64. That means that
performance profiling by application developers looks grouped among
seemingly unrelated code paths.
Related to GNOME/Initiatives#10
We already have the GType with which the GValue should be initialized,
so requiring an initialized GValue is not really necessary, and it
actually complicates code that wraps GObject, by requiring the retrieval
of the GParamSpec in order to get the property type. Additionally, it
introduces a mostly unnecessary g_value_reset().
We already changed g_object_getv() to allow passing uninitialized
GValues, but this fell through the cracks.
Closes: #737
These have all been documented as deprecated for a long time, but we’ve
never had a way to programmatically mark them as deprecated. Do that
now.
This is based on the list of deprecations from the reverted commit
80fcb1bc2.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #638
g_object_set_data() should only ever be used with a small, bounded set
of keys, or the memory usage of the quark lookup table will grow
unbounded. Document that.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #682
I'm trying to use `-fsanitize=thread` for OSTree, and some of
these issues seem to go into GLib. Also, the sanitizers work better if
the userspace libraries are built with them too.
This fix is similar to
b6814bb37c
Mixing atomic and non-atomic reads trips TSAN, so let's change the
assertions to operate on the local values returned from atomic
read/writes.
Without this change I couldn't even *build* GLib with TSAN, since we
use gresources during compilation, which uses GSubprocess, which hits
this code.
(Minor review fixes made by Philip Withnall <withnall@endlessm.com>.)
https://gitlab.gnome.org/GNOME/glib/issues/1224
An assertion is harder to skip over, and using a g_critical() can give
us a more informative error message.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/179
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__().
There is no transfer annotation that can express transfer semantics of
g_object_new_with_properties in general. When GInitiallyUnowned object
is constructed the introspection data will be incorrect.
Mark it with skip annotation.
https://bugzilla.gnome.org/show_bug.cgi?id=795025
Currently, g_object_ref() and g_object_ref_sink() return a
gpointer which can mask issues when assigning to fields or
returning from a function.
To help catch these type of programming errors, we can propagate
the type of the parameter through the function call on GCC
using the typeof() C language extension.
This will cause offending code to have a warning, but will
continue to be source and binary compatible.
This is only enabled when GLIB_VERSION_MAX_ALLOWED is 2.56 or greater.
https://bugzilla.gnome.org/show_bug.cgi?id=790697
Where we were already treating GHashTables as sets, modify them to use
the set-specific APIs g_hash_table_add() and g_hash_table_contains(), to
make that usage more obvious and less prone to being broken.
Heavily based on patches by Garrett Regier <garrettregier@gmail.com>.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=749371
This was duplicated also in g_object_interface_install_property().
Now, validations specific to classes happen in
validate_and_install_class_property() - specifically, the checks for
the presence of the get_property() and set_property() methods.
https://bugzilla.gnome.org/show_bug.cgi?id=787551
It's unnecessary, and only adds visual noise; we have been fairly
inconsistent in the past, but the semi-colon-less version clearly
dominates in the code base.
https://bugzilla.gnome.org/show_bug.cgi?id=669355
This is going to be checked again by g_object_new_with_properties()
and g_object_new_valist() anyway, so might just as well leave it
to those functions to do the check and only do it once. It doesn't
matter which function emits the critical warning in the end either,
as one has to look at a stack trace to find out what code triggered
it in any case.
https://bugzilla.gnome.org/show_bug.cgi?id=780908
g_object_newv uses a GParameter as argument. Since GParameter
is deprecated due to this type is not introspectible,
g_object_newv is deprecated now.
https://bugzilla.gnome.org/show_bug.cgi?id=709865
g_object_new_with_properties is an alternative to g_object_newv.
The last one, takes an array of GParameter. However, GParameter
is a rarely used type and this type is not introspectible, so
it will not work properly in bindings.
https://bugzilla.gnome.org/show_bug.cgi?id=709865
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.
It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
The C spec leaves conditional evaluation inside a macro expansion as
undefined behaviour. This means we cannot use constructs like:
GOBJECT_IF_DEBUG(OBJECTS, {
...
#ifdef BLAH
...
#endif
...});
Because compilers are entirely justified to ignore the conditional, or,
like in the case of MSVC, error out.
https://bugzilla.gnome.org/show_bug.cgi?id=769504
historically, DEBUG_CODE(gtype.c) and IF_DEBUG(gobject.c, gsignal.c)
macros are used to support debugging messages about object bookkeeping
and signal emission.
DEBUG_CODE has never been used in gtype.c. IF_DEBUG, when used, must be
accompanied by an extra #ifdef G_ENABLE_DEBUG. this is cumbersome.
this patch add a new macro GOBJECT_IF_DEBUG based on DEBUG_CODE as
a replacement for both DEBUG_CODE and IF_DEBUG.
https://bugzilla.gnome.org/show_bug.cgi?id=729914
Add various (nullable) and (optional) annotations which were missing
from a variety of functions. Also port a couple of existing (allow-none)
annotations in the same files to use (nullable) and (optional) as
appropriate instead.
Secondly, add various (not nullable) annotations as needed by the new
default in gobject-introspection of marking gpointers as (nullable). See
https://bugzilla.gnome.org/show_bug.cgi?id=729660.
This includes adding some stub documentation comments for the
assertion macro error functions, which weren’t previously documented.
The new comments are purely to allow for annotations, and hence are
marked as (skip) to prevent the symbols appearing in the GIR file.
https://bugzilla.gnome.org/show_bug.cgi?id=719966
Keeping these enabled causes too many people to file
bugs against gobject, and not enough people to send
patches to port away from deprecated properties.
Practically no caller of these functions require atomic behaviour,
but the atomics are much slower than normal operations, which makes
it desirable to get rid of them. We have not done this before because
that would be a break of the ABI.
However, I recently looked into this and it seems that even if the
atomics *are* used for g_clear_* it is not ever safe to use this. The
atomics protects two threads that are racing to free a global/shared
object from freeing the object twice. However, any *user* of the global
object have no protection from the object being freed while in use,
because there is no paired operation the reads and refs the object
as an atomic unit (nor can such an operation be implemented using
purely atomic ops).
So, since nothing could safely have used the atomic aspects of these
functions I consider it acceptable to just remove it.
https://bugzilla.gnome.org/show_bug.cgi?id=733969
Don't emit property deprecation warnings for construct properties that
are being set to their default value during construction, but _do_ emit
them in all cases when the property was explicitly given to
g_object_new().
https://bugzilla.gnome.org/show_bug.cgi?id=732184
By default G_PARAM_DEPRECATED means absolutely nothing. We only emit a
warning if G_ENABLE_DIAGNOSTIC is set to '1' and then, only on sets.
Turn the logic on its head: emit the warning by default, unless
G_ENABLE_DIAGNOSTIC is set to 0. In order to avoid a torrent of output, only
emit a warning once per property name.
https://bugzilla.gnome.org/show_bug.cgi?id=732184
Leave ourselves a little wiggle room: if people install properties after
initialisation then we reserve the right to handle that in a way that
may not be threadsafe.
https://bugzilla.gnome.org/show_bug.cgi?id=698614
