We only need to take a ref on the object when
we call out to external code (ie around
->dispatch_properties_changed). If we avoid
the signal emission, we can avoid the ref/unref
too. This is not currently happening, but
might in the future.
A small reorg that reduces the code and matches
what we do for object_get_property.
Note that as a consequence of this change, we now
check the deprecated flag on the redirected property,
not on the original when setting properties. This
matches what we were already doing for getting
properties.
The code that emits property deprecation warnings
rarely runs, and doesn't need to be inlined
everywhere. It is enough to inline the check for
the deprecation flag.
It is safe not to copy arguments here,
because we are not emitting any signals
before we are done setting the values
as properties.
This matches what we do for g_object_new now.
We can safely use the values without copying here.
This is safe because we are not emitting any
signals before we are done setting the values
as properties.
These have all been added manually, as I’ve finished all the files which
I can automatically detect.
All the license headers in this commit are for LGPL-2.1-or-later, and
all have been double-checked against the license paragraph in the file
header.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
This avoids walking the construct params list
one extra time just to count when constructing
objects, for a small speedup of object construction
in the presence of construct params.
Avoid GValue transformation when we can, using
the new value_is_valid vfunc.
This is particularly useful for string properties,
where g_value_transform will make a copy of the string.
In several places we do paired calls of g_value_init
and g_value_unset, both of which peek the value table.
We can avoid half of that cost by remembering the value
table, instead of looking it up again.
This uses the new G_VALUE_COLLECT_INIT2 macro.
Drop the redundant `PROP_0` (which isn’t a real property) and initialise
the first member of the enum instead.
Add a typedef so that the enum type can be used in `switch` statements
in `get_property()` and `set_property()` vfuncs. This allows
`-Wswitch-enum` to be used to improve type safety.
The examples here don’t have `get_property()` or `set_property()`
vfuncs, but people might copy/paste the code to somewhere which does.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Make it a bit clearer in the documentation that using
`G_PARAM_STATIC_STRINGS` everywhere is a good thing.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Notifying during object destruction is a dubious "feature": objects
might end up recreating a bunch of state just before clearing it;
language bindings might get spurious notifications during garbage
collection runs.
We freeze the notification queue before running the dispose() chain; if
the object was temporarily vivified during dispose, we thaw the
notification queue, otherwise we let the instance clear it when we
finalize it.
See: https://gitlab.gnome.org/GNOME/gjs/-/issues/445
We now guarantee that GObjects will always be allocated at least as
aligned as the basic types. If you want to put an element in your
GObject which has higher alignment requirements, we can’t guarantee it
will be aligned*. If you need it to be aligned, you’ll need to put it on
the heap (aligned appropriately), or add appropriate padding in your
GObject struct.
*Actually, GSlice will guarantee that the whole GObject is aligned to at
least the power of 2 greater than or equal to the size of the GObject,
which means any element in the GObject struct should always be
appropriate aligned if the compiler pads it appropriately. If malloc()
is used, however, it doesn’t make that guarantee, so we can’t make that
guarantee overall.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1231
See the reasoning in the patch for why we believe GObjects *are*
(already) as aligned as the basic types.
We want to make this guarantee so that it’s guaranteed to be safe for
people to ignore -Wcast-align warnings for GObjects which contain basic
types. This typically happens with gdouble on 32-bit ARM platforms.
The checks are slightly complicated by the need to support GObjects with
custom constructors. We should expect that a custom construction
function will chain up to g_object_constructor (which calls
g_type_create_instance() as normal), but it’s possible that someone has
done something crazy and uses a custom allocator which doesn’t return
with the same alignment as GSlice. Hand them a warning in that case. If
that is true, the code which uses their custom-constructed GObject can
presumably already deal with the alignment it gets given.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1231
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
Add a flag to prevent the automatic emission of the "notify" signal
during g_object_set_property().
If this flag is set then the class must explicitly emit the notify
for themselves. This is already standard practice on most classes, but
we cannot simply remove the existing behaviour because there are surely
many cases where it is needed.
https://bugzilla.gnome.org/show_bug.cgi?id=731200
Construct properties are always set during construction.
It makes no sense to warn about this even if the property
is marked as deprecated; the deprecation warning should
only be issues for explicit uses of the property after
construction.
https://bugzilla.gnome.org/show_bug.cgi?id=730045
Although returning NULL from constructor is strongly discouraged, some
old libraries need to keep doing it for ABI-compatibility reasons.
Given this, it's rude to forbid finalization from within
constructor(), since it would otherwise work correctly now anyway (and
the critical when returning NULL should discourage any new uses of
returning NULL from constructor()).
https://bugzilla.gnome.org/show_bug.cgi?id=661576
Since we are no longer using sgml mode, using /* */ to
escape block comments inside examples does not work anymore.
Switch to using line comments with //
This is really just a very crude and limited conditional breakpoint.
Update the documentation to explain conditional breakpoints in
gdb instead. Also, remove the link to refdbg, which appears dead.
https://bugzilla.gnome.org/show_bug.cgi?id=719687
The signals queued while notify is frozen are emitted in
reverse order, while omitting duplicates. The lack of documentation
for this was pointed out in
https://bugzilla.gnome.org/show_bug.cgi?id=607016
Rather than keeping a global list of objects that are being
constructed, use qdata on the object itself like we do with several
other properties now.
https://bugzilla.gnome.org/show_bug.cgi?id=661576
If a constructor() implementation created an object but then unreffed
it rather than returning it, that object would get left on the
construction_objects list, which would cause problems later when that
memory location got reused by another object.
"Fix" this by making it fail intentionally, and add a test for it (and
for the normal, working singleton case).
https://bugzilla.gnome.org/show_bug.cgi?id=661576
Just like g_object_notify, check for a zero ref_count in
g_object_notify_by_pspec and leave if it is 0.
This allows using functions in ->finalize() that possibly also
notify a property change on the object. Previously,
this resulted in an error from g_object_ref.
https://bugzilla.gnome.org/show_bug.cgi?id=705570
We have turned up enough cases of this being done (including GTK API
allowing apps to do this to GtkSettings well after it has been
instantiated) that it is clear that we cannot really break this feature
while claiming to be backwards compatible.
For that reason, it becomes a warning rather than a critical (ie: it is
still well-defined behaviour, but you are discouraged from doing it).
The intention is to keep this feature for at least the next while.
A given GObjectClass will be able to avoid using GParamSpec pool for as
long as you don't install properties after init. If you do that, you
will get a warning and we will devolve to using GParamSpecPool.
https://bugzilla.gnome.org/show_bug.cgi?id=698614
GObject has previously allowed installing properties after class_init
has finished running. This means that you could install some of your
own properties on G_TYPE_OBJECT, for example, although they wouldn't
have worked properly.
A previous patch asserted that this was not true and we had to revert it
because it broke the shell. Instead of reverting, we should have used a
critical, so do that now.
Complaints go to this bug:
https://bugzilla.gnome.org/show_bug.cgi?id=698614
