This change was previously implemented in
9ba17d511e but got dropped during the
Python conversion of the Perl script.
See the commit message of this commit as well as
https://bugzilla.gnome.org/show_bug.cgi?id=782162
for more information.
This patch also adds a new test so we don't loose this feature again.
Also adds a test that checks that the G_SIGNAL_RUN flags are handled
correctly and the class signal handler is called at the right times.
Fixes https://gitlab.gnome.org/GNOME/glib/issues/513
gobject/tests/ifaceproperties.c: In function ‘base_object_get_type’:
gobject/tests/ifaceproperties.c:321:1: error: missing initializer for field ‘value_table’ of ‘GTypeInfo’ {aka ‘const struct _GTypeInfo’}
321 | static DEFINE_TYPE_FULL (BaseObject, base_object,
| ^~~~~~
In file included from gobject/gobject.h:24,
from gobject/gbinding.h:29,
from glib/glib-object.h:22,
from gobject/tests/ifaceproperties.c:21:
gobject/gtype.h:1063:26: note: ‘value_table’ declared here
1063 | const GTypeValueTable *value_table;
| ^~~~~~~~~~~
gobject/tests/ifaceproperties.c: In function ‘test_iface_get_type’:
gobject/tests/ifaceproperties.c:144:1: error: missing initializer for field ‘class_finalize’ of ‘GTypeInfo’ {aka ‘const struct _GTypeInfo’}
144 | static DEFINE_IFACE (TestIface, test_iface, NULL, test_iface_default_init)
| ^~~~~~
In file included from gobject/gobject.h:24,
from gobject/gbinding.h:29,
from glib/glib-object.h:22,
from gobject/tests/ifaceproperties.c:21:
gobject/gtype.h:1054:26: note: ‘class_finalize’ declared here
1054 | GClassFinalizeFunc class_finalize;
| ^~~~~~~~~~~~~~
gobject/tests/signals.c: In function ‘test_introspection’:
gobject/tests/signals.c:1180:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
1180 | for (i = 0; i < n_ids; i++)
| ^
gobject/tests/properties.c: In function ‘properties_get_property’:
gobject/tests/properties.c:562:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’
562 | for (i = 0; i < G_N_ELEMENTS (test_props); i++)
| ^
gobject/tests/properties.c:583:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’
583 | for (i = 0; i < G_N_ELEMENTS (test_props); i++)
| ^
gobject/tests/dynamictests.c: In function ‘test_module_get_type’:
gobject/tests/dynamictests.c:97:7: error: missing initializer for field ‘value_table’ of ‘GTypeInfo’ {aka ‘const struct _GTypeInfo’}
97 | };
| ^
In file included from gobject/gobject.h:24,
from gobject/gbinding.h:29,
from glib/glib-object.h:22,
from gobject/tests/dynamictests.c:23:
gobject/gtype.h:1063:26: note: ‘value_table’ declared here
1063 | const GTypeValueTable *value_table;
| ^~~~~~~~~~~
gobject/gparam.c: In function ‘g_param_type_register_static’:
gobject/gparam.c:1434:3: error: missing initializer for field ‘value_table’ of ‘GTypeInfo’ {aka ‘struct _GTypeInfo’}
1434 | };
| ^
In file included from gobject/gvalue.h:26,
from gobject/gparam.h:26,
from gobject/gparam.c:26:
gobject/gtype.h:1063:26: note: ‘value_table’ declared here
1063 | const GTypeValueTable *value_table;
| ^~~~~~~~~~~
gobject/gtypemodule.c: In function ‘g_type_module_get_type’:
gobject/gtypemodule.c:154:7: error: missing initializer for field ‘value_table’ of ‘GTypeInfo’ {aka ‘const struct _GTypeInfo’}
154 | };
| ^
In file included from gobject/gtypeplugin.h:24,
from gobject/gtypemodule.c:22:
gobject/gtype.h:1063:26: note: ‘value_table’ declared here
1063 | const GTypeValueTable *value_table;
| ^~~~~~~~~~~
gobject/gtypeplugin.c: In function ‘g_type_plugin_get_type’:
gobject/gtypeplugin.c:91:7: error: missing initializer for field ‘class_init’ of ‘GTypeInfo’ {aka ‘const struct _GTypeInfo’}
91 | };
| ^
In file included from gobject/gtypeplugin.h:24,
from gobject/gtypeplugin.c:20:
gobject/gtype.h:1053:26: note: ‘class_init’ declared here
1053 | GClassInitFunc class_init;
| ^~~~~~~~~~
gobject/gtype.c: In function ‘iface_node_has_available_offset_L’:
gobject/gtype.c:1288:42: error: comparison of integer expressions of different signedness: ‘gsize’ {aka ‘long unsigned int’} and ‘int’
1288 | if (G_ATOMIC_ARRAY_DATA_SIZE (offsets) <= offset)
| ^~
The version of `black` on the CI server wanted these changes. Make them
to keep the `style-check-diff` CI job from constantly failing.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
This commit only looks at the `Returns:` lines in the documentation, and
has examined all of them in the file. Function arguments have not been
checked.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2227
When this is called on the source or target, the weak notify of the
corresponding object is called without the GWeakRef being cleared.
See https://gitlab.gnome.org/GNOME/glib/-/issues/2266 for that issue.
This means that a strong reference to these zombie objects can be
retrieved from the GWeakRefs and the previous assumption that this can't
happen was wrong. Remove the assertion for that accordingly and handle
this case.
Specifically, all signal handlers and weak notifies of the object are
already gone and must not be disconnected/removed a second time, or
otherwise memory corruption would be caused. Instead just set the
GWeakRef to NULL and handle it otherwise as if the GWeakRef didn't give
a strong reference to begin with.
Fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2265
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/tests/value.c: In function ‘test_valuearray_basic’:
gobject/tests/value.c:253:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
253 | for (i = 0; i < a->n_values - 1; i++)
| ^
gobject/tests/value.c:257:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘guint’ {aka ‘unsigned int’}
257 | for (i = 0; i < a->n_values; i++)
| ^
This was inconsistently handled before and only explicit unbinding or
finalizing the binding would've previously released the transform
function. If the source/target were finalized while more strong
references to the binding still existed then the transform function
would stay alive and only the binding itself would be deactivated.
Unbinding can happen from one thread while a property notification is
being handled concurrently in another one.
To solve this, introduce a reference counter for the transform function
that ensures that it always stays valid while in use and protect access
to the one stored inside the binding with the unbind mutex.
It's possible for g_binding_unbind() to be called at the same time as
one (or both) of source and target are being finalized. The resulting
unbinding needs to be protected with a mutex to ensure that it only
happens exactly once.
As the first reference is owned by both weak notifies and the caller of
g_object_bind_property(), additional indirections are needed to ensure that
unreffing the first reference after creation still unbinds the binding
as before. This seems to be a common code pattern and how this was
intended to be used, but is only safe in single-threaded contexts as it
relies on both the source and target object to be still alive.
Add a lot of comments to the code about all these dependencies and a
couple of assertions to ensure they hold valid.
Also document that inconsistent reference ownership handling of
g_binding_unbind() that makes it unfit for automatically generated
language bindings.
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++)
| ^
gobject/gtype.c: In function ‘g_type_interface_add_prerequisite’:
gobject/gtype.c:1607:21: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘int’
1607 | for (i = 0; i < prerequisite_node->n_supers + 1; i++)
| ^
The problem occurs because we keep a pointer inside the allocated block,
instead of a pointer to the start of the block. This memory exists for
the lifetime of the application, so let's silence it.
This is probably abuse of VALGRIND_MALLOCLIKE_BLOCK(), which is really
intended for use in memory allocators, but gtype.c already uses it in
two other places, and it's a practical solution. I wrote another larger
fix for this issue that involves keeping an array of extra pointers when
running under valgrind. This is simpler.
Fix suggested by Philip Withnall
```
==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,078 of 16,075
==180238== at 0x483BB1A: calloc (vg_replace_malloc.c:762)
==180238== by 0x5489495: g_malloc0 (gmem.c:132)
==180238== by 0x5489754: g_malloc0_n (gmem.c:364)
==180238== by 0x53FDBEE: type_set_qdata_W (gtype.c:3722)
==180238== by 0x53FDEE8: type_add_flags_W (gtype.c:3787)
==180238== by 0x53FC348: g_type_register_fundamental (gtype.c:2662)
==180238== by 0x53D969B: _g_enum_types_init (genums.c:124)
==180238== by 0x53FF058: gobject_init (gtype.c:4432)
==180238== by 0x53FF082: gobject_init_ctor (gtype.c:4493)
==180238== by 0x4010F29: call_init.part.0 (dl-init.c:72)
==180238== by 0x4011030: call_init (dl-init.c:30)
==180238== by 0x4011030: _dl_init (dl-init.c:119)
==180238== by 0x4002149: ??? (in /usr/lib64/ld-2.30.so)
```
Fixes#2076
The problem occurs because we keep a pointer inside the allocated block,
instead of a pointer to the start of the block:
```
==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,086 of 16,075
==180238== at 0x483980B: malloc (vg_replace_malloc.c:309)
==180238== by 0x548942C: g_malloc (gmem.c:102)
==180238== by 0x54A4748: g_slice_alloc (gslice.c:1025)
==180238== by 0x53D0AAF: freelist_alloc (gatomicarray.c:77)
==180238== by 0x53D0B85: _g_atomic_array_copy (gatomicarray.c:133)
==180238== by 0x53F8E6D: iface_node_set_offset_L (gtype.c:1347)
==180238== by 0x53F91F1: type_node_add_iface_entry_W (gtype.c:1444)
==180238== by 0x53F93DF: type_add_interface_Wm (gtype.c:1477)
==180238== by 0x53FC946: g_type_add_interface_static (gtype.c:2852)
==180238== by 0x4A3D53A: gtk_menu_shell_accessible_get_type_once (gtkmenushellaccessible.c:26)
==180238== by 0x4A3D495: gtk_menu_shell_accessible_get_type (gtkmenushellaccessible.c:26)
==180238== by 0x4C8AC44: gtk_menu_shell_class_init (gtkmenushell.c:424)
```
Note we cannot use VALGRIND_FREELIKE_BLOCK() in freelist_free() because we
have not actually freed the FreeListNode and need to dereference it in
freelist_alloc() to decide whether to reuse the block. That would result
in a use-after-free warning before we would get a chance to call
VALGRIND_MALLOCLIKE_BLOCK() in the reuse path.
Also note that this free list only ever grows: it never shrinks for the
lifetime of the application, so nothing here will ever be truely freed,
although unused elements are eligible for reuse.
Fix suggested by Philip Withnall
Related: #2076
gobject/gtype.c: In function ‘type_node_add_iface_entry_W’:
gobject/gtype.c:1379:21: error: comparison of integer expressions of different signedness: ‘guint’ {aka ‘unsigned int’} and ‘int’
1379 | for (i = 0; i < num_entries; i++)
| ^
gobject/gtype.c: In function ‘lookup_iface_entry_I’:
gobject/gtype.c:599:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’
599 | if (index < IFACE_ENTRIES_N_ENTRIES (entries))
| ^
gobject/gatomicarray.h:51:8: note: in definition of macro ‘G_ATOMIC_ARRAY_DO_TRANSACTION’
51 | {_C_;} \
| ^~~
Half of the references to `init_state` in `gtype.c` already correctly
accessed it atomically, but a couple didn’t. Drop the `volatile`
qualifier from its declaration, as that’s not necessary for atomic
access.
Note that this is the `init_state` in `TypeData`, *not* the `init_state`
in `IFaceEntry`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
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
This is an API break, but no third party code should be touching
`GObject.ref_count`, let alone in a way which would be changed by the
removal of the `volatile` qualifier.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
This is an API break, but it should not affect third party code since
that code should not be interacting with the `data` member in a way that
invokes its `volatile` qualifier (such as copying to an intermediate
variable).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #600
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
http://isvolatileusefulwiththreads.in/c/
It’s possible that the variables here are only marked as volatile
because they’re arguments to `g_once_*()`. Those arguments will be
modified in a subsequent commit.
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.
