These are similar to g_pointer_bit_lock_and_get() and
g_pointer_bit_unlock_and_set().
The bitlock API is pretty amazing, as it allows to use a single bit of
an integer for locking while using the remaining 31 bits for other
purposes. But those other bits always need to be accessed atomically
too.
There is a use in being able to lock_and_get(), to combine the setting
of the lock bit and fetching the new value at once. For one, that can
safe additional atomic operations to fetch the value afterwards. But
more importantly, it allows to do this change in one atomic operation.
Likewise, unlock_and_set() allows to atomically clear the lock bit and
set a new value (while also preserving unrelated bits, by using the
@preserve_mask parameter).
Spotted while looking at CI output for the previous commit, and I
thought I might as well fix it while I was there.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This should fix compile errors on macOS, which couldn’t find the
`write()`, `lseek()`, etc. functions added in commit
020e58bac53e2d06e27cb1ab85d488e743abe9b4.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
It was always passed the same value by all users of the macro.
This introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Rather than atomically querying the flags, and then atomically reffing
the `GClosure`, do the atomic ref and return the new flags value as a
side-effect. This eliminates the need for an additional atomic read, and
eliminates a race window between the two accesses.
It does mean we need a new internal version of `g_closure_ref()` which
returns the new `GClosureFlags` though.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Remove the last few non-atomic reads of `GClosure.ref_count`. See
previous commits for an explanation of why this is important.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Currently, `GClosure` does atomic writes to the flags in its initial
bitfield, but it never does atomic reads from them. This is not safe —
even if the memory read for an int is atomic, the compiler might
duplicate or rearrange a non-atomic read and give an unexpected result.
Using an atomic fetch inserts the necessary compiler and hardware
barriers to ensure a single result is fetched.
(See !4575 of an example of where this kind of bug has caused
misbehaviour.)
So, introduce an atomic read helper for the `GClosure` bitfield. Rather
than reading a single member of the bitfield, it returns the entire
bitfield, as several `GClosure` methods need access to multiple members
of the bitfield, and atomically fetching them all separately would not
be atomic overall.
Fix various `GClosure` methods to use the new atomic read function.
There are more parts of the code still to fix — these were just the
straightforward ones.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Depending on luck, g_closure_ref may access closure->vint and observe
different values between reads. This manifests as a test failure in
signals-refcount{2,4}, properties-refcount1, and closure-refcount depending
on timing and re-runs.
Jakub Jelinek analysed this on GCC bug PR119607 after I'd reported it
over there as a possible GCC regression.
The critical part being g_closure_ref -> ATOMIC_INC_ASSIGN -> ATOMIC_CHANGE_FIELD
where closure->vint gets re-read repeatedly, both outside and inside the retry
loop. To fix that:
1. Atomically fetch it the first time;
2. Use the cached read, not a fresh read, of vint in the loop;
3. Use g_atomic_int_compare_and_exchange_full in the loop so we get a freshly
cached vint if it changed in another thread.
Bug: https://gcc.gnu.org/PR119607
Fixes: 834ddd19 ('turned all modifications to the first 32 integer bits in a closure into')
Co-authored-by: Jakub Jelinek <jakub@redhat.com>
`GDate` currently has some API which is specific to which day of the week
you think the week starts on:
* `g_date_get_monday_week_of_year()`
* `g_date_get_monday_weeks_in_year()`
* `g_date_get_sunday_week_of_year()`
* `g_date_get_sunday_weeks_in_year()`
This is to deal with the difference between locales which think that the
week starts on a Sunday (such as `LANG=en_US.utf8` or `LANG=he_IL.utf8`),
or a Monday (such as `LANG=en_GB.utf8`).
However, there are some locales which think that the week starts on a
Saturday (such as `LANG=ar_EG.utf8`). Currently, GLib provides no API for
those.
So, add some API which is parameterised by the first day of the week,
which will deal with weeks which start on a Saturday (and also any other
day of the week, although I don’t believe there are any countries which
use a day other than Saturday, Sunday or Monday).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3617
We had code to avoid that we could call a toggle "up" notification
callback in locked state, but this was not covering the case in which
the cancellable second to last reference was removed in its cancellation
callback.
In fact, in such case we end up going from 2 -> 1 references during the
signal callback call and this leads to calling the toggle notify
callback in locked state.
To prevent this, add an even further reference before calling the
callback (in locked state, but there's no risk that a toggle-up
notification happens now), and drop it once unlocked again.
If when calling g_cancellable_connect() the cancellable was already
cancelled we could have ended up in calling the data cleanup
function while the cancellable lock was held.
This is likely not an issue, but it's still better not to do it,
so protect the code against it
We had wrong comments, in particular:
@callback is called exactly once each time @cancellable is cancelled,
either directly at the time of the connect if @cancellable is already
cancelled, or when @cancellable is cancelled in some thread.
In case the cancellable is reset via [method@Gio.Cancellable.reset]
then the callback can be called again if the @cancellable is cancelled
This is not true if `@cancellable` is already cancelled because then the
callback will be thrown away, and will not be called for subsequent
cancellations.
Since GLib 2.40, the lock protecting @cancellable is not held when
@callback is invoked. This lifts a restriction in place for
earlier GLib versions which now makes it easier to write cleanup
code that unconditionally invokes e.g. g_cancellable_cancel().
As per commit 3a07b2abd4006 this section needs some amending since
it's not true anymore, as the GCancellable's mutex is held when `@callback` is
invoked. However this happens in a way that it's still safe to call most
of the GCancellable API.
Closes: #3642
When a non-cancelled cancellable ::cancelled signal callback is called
the cancellable has enough references so that it can be unreferenced on
the callback itself. However this doesn't happen if the cancellable has
been already cancelled at the moment we connect to it.
To prevent this, add a temporary reference before calling the signal
callback.
Note that we do this also if the callback has not been already cancelled
to prevent that we may end up calling a toggle-notify callback while we
are locked.
Add tests
Closes: #3643
In function ‘memmem_with_end_pointer’,
inlined from ‘domain_found’ at ../glib/gmessages.c:2695:16,
inlined from ‘should_drop_message’ at ../glib/gmessages.c:2802:35:
../glib/gmessages.c:2674:19: warning: ‘log_domain_length’ may be used
uninitialized [-Wmaybe-uninitialized]
2674 | #define my_memmem memmem
| ^
../glib/gmessages.c:2683:10: note: in expansion of macro ‘my_memmem’
2683 | return my_memmem (haystack, (const char *) haystack_end - (const char *) haystack, needle, needle_len);
| ^~~~~~~~~
../glib/gmessages.c: In function ‘should_drop_message’:
../glib/gmessages.c:2760:13: note: ‘log_domain_length’ was declared here
2760 | gsize log_domain_length;
| ^~~~~~~~~~~~~~~~~
If a value has no size, we'd end up to use g_variant_store() with a NULL
value, and this is not allowed by the API, leading to errors:
stderr:
../glib/gvariant-core.c:1429:9: runtime error: null pointer passed as argument
1, which is declared to never be null
#0 0x7f9c139ce559 in g_variant_store ../glib/gvariant-core.c:732
#1 0x7f9c13c60b01 in g_variant_byteswap ../glib/gvariant.c:6211
#2 0x564ae412e9b9 in test_byteswap ../glib/tests/gvariant.c:2321
#3 0x564ae412e9b9 in test_byteswaps ../glib/tests/gvariant.c:2374
#4 0x7f9c13bc1000 in test_case_run ../glib/gtestutils.c:3115
#5 0x7f9c13bc1000 in g_test_run_suite_internal ../glib/gtestutils.c:3210
#6 0x7f9c13bc0d7b in g_test_run_suite_internal ../glib/gtestutils.c:3229
#7 0x7f9c13bc0d7b in g_test_run_suite_internal ../glib/gtestutils.c:3229
#8 0x7f9c13bc2019 in g_test_run_suite ../glib/gtestutils.c:3310
#9 0x7f9c13bc216f in g_test_run ../glib/gtestutils.c:2379
#10 0x564ae410f326 in main ../glib/tests/gvariant.c:6045
In case data is NULL we ended up to call memcpy with NULL parameter
which is undefined behavior (see the trace below).
So instead of having multiple null checks to do just the same, simplify
the NULL or 0-sized cases.
../glib/gbytes.c:140:7: runtime error: null pointer passed as argument 2,
which is declared to never be null
#0 0x7f56ea7c667e in g_bytes_new ../glib/gbytes.c:140
#1 0x5557c3659f06 in test_null ../glib/tests/bytes.c:453
#2 0x7f56ea9c0f70 in test_case_run ../glib/gtestutils.c:3115
#3 0x7f56ea9c0f70 in g_test_run_suite_internal ../glib/gtestutils.c:3210
#4 0x7f56ea9c0ceb in g_test_run_suite_internal ../glib/gtestutils.c:3229
#5 0x7f56ea9c1f89 in g_test_run_suite ../glib/gtestutils.c:3310
#6 0x7f56ea9c20df in g_test_run ../glib/gtestutils.c:2379
#7 0x5557c36599d2 in main ../glib/tests/bytes.c:536
Ensure we don't do an user-after-free access, as reported by ASAN:
==3704==ERROR: AddressSanitizer: stack-use-after-return on address
0x70a58f8631c0 at pc 0x000000405144 bp 0x7fffff62c7a0 sp 0x7fffff62c798
READ of size 4 at 0x70a58f8631c0 thread T0
#0 0x405143 in on_object_unregistered ../../GNOME/glib/gio/tests/gdbus-export.c:597
#1 0x70a592e858d8 in call_destroy_notify_data_in_idle ../../GNOME/glib/gio/gdbusconnection.c:244
#2 0x70a5940016a4 in g_idle_dispatch ../../GNOME/glib/glib/gmain.c:6221
#3 0x70a59401095b in g_main_dispatch ../../GNOME/glib/glib/gmain.c:3348
#4 0x70a59401095b in g_main_context_dispatch_unlocked ../../GNOME/glib/glib/gmain.c:4197
#5 0x70a59401ba17 in g_main_context_iterate_unlocked ../../GNOME/glib/glib/gmain.c:4262
#6 0x70a59401cc73 in g_main_context_iteration ../../GNOME/glib/glib/gmain.c:4327
#7 0x405658 in test_threaded_unregistration_iteration ../../GNOME/glib/gio/tests/gdbus-export.c:1878
#8 0x405658 in test_threaded_unregistration ../../GNOME/glib/gio/tests/gdbus-export.c:1952
#9 0x70a5940dfb04 in test_case_run ../../GNOME/glib/glib/gtestutils.c:2988
#10 0x70a5940dfb04 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3090
#11 0x70a5940df893 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3109
#12 0x70a5940df893 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3109
#13 0x70a5940e0bc9 in g_test_run_suite ../../GNOME/glib/glib/gtestutils.c:3189
#14 0x70a5940e0d1f in g_test_run ../../GNOME/glib/glib/gtestutils.c:2275
#15 0x40eb72 in session_bus_run ../../GNOME/glib/gio/tests/gdbus-sessionbus.c:69
#16 0x403a2c in main ../../GNOME/glib/gio/tests/gdbus-export.c:1990
#17 0x70a591d9f149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2)
#18 0x70a591d9f20a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2)
#19 0x403b44 in _start (/tmp/_build/gio/tests/gdbus-export+0x403b44) (BuildId: f6312e919c3d94e4c49270b0dfc5c870e1ba550b)
Address 0x70a58f8631c0 is located in stack of thread T0 at offset 192 in frame
#0 0x40525f in test_threaded_unregistration ../../GNOME/glib/gio/tests/gdbus-export.c:1936
This frame has 7 object(s):
[32, 40) 'local_error' (line 1835)
[64, 72) 'unregister_thread' (line 1836)
[96, 104) 'value' (line 1838)
[128, 136) 'value_str' (line 1839)
[160, 168) 'call_result' (line 1840)
[192, 204) 'object_registration_data' (line 1834) <== Memory access at offset 192 is inside this variable
[224, 240) 'data' (line 1833)
This is what we already ignored with valgrind and it's something that
we should always ignore since it's leaked by design.
So do explicitly mark it as such.
