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
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
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)
Rather than creating files in the current directory. This is a bit
neater, and avoids races between parallel invocations of the unit tests
if the file names aren’t guaranteed to be unique (e.g. by using
`g_mkstemp()`).
Add `G_TEST_OPTION_ISOLATE_DIRS` too, to make sure we use a unique
subdirectory of `g_get_tmp_dir()`. This means that paths like
`g_get_tmp_dir() / some-file` are guaranteed to be race-free even if the
filename is not unique, because the test tmp dir now is.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
In the `g-file-info-filesystem-readonly` test.
This doesn’t introduce any functional changes, but makes the code a
little easier to read (because the parts of the path are now in
hierarchical order) and makes it a bit clearer that we’re building a
path rather than an arbitrary string.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
It’s not entirely clear from the documentation, but `g_mkstemp()` (and
`g_mkdtemp()`) operate in the current directory, rather than the system
temporary directory.
This meant these tests were all writing files to the build directory.
This is messy, though thankfully not a correctness issue or a race
because `g_mkstemp()` guarantees to return a unique file for each
caller.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Like many things I touch, I broke this in
fd8ede0b661aa67032bbc3e7afc88aff22d7984a.
Spotted by Sebastian Wilhelmi in
fd8ede0b66 (note_2385263).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Its symbol interposition works differently to that of Linux, so our
approach using `dlsym(RTLD_NEXT)` to inject syscalls (and still allow
chaining up to the version from libc) doesn’t work on macOS.
See https://gitlab.gnome.org/GNOME/glib/-/jobs/4861349 for an example
failure.
It would be lovely to have these tests working on macOS, but I am not a
macOS developer, and have spent enough time fixing this leak (#1250)
already. It can wait for follow-up work.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1250
The algorithm that `g_socket_listener_add_any_inet_port()` and
`g_socket_listener_add_inet_port()` use to try to connect to IPv4 and/or
IPv6 ports are a bit complex (especially when port allocation has to
happen in the former method). So far they’ve not really been unit
tested, which is unfortunate, and has left latent bugs.
Add some unit tests for both methods, by providing mock `socket()` (and
friends) functions to override those from libc, and using those to cause
specific syscalls to fail according to the test’s needs.
These tests demonstrate the fix for #1250 works, as the tests can be run
under memcheck and show no memory leaks. They’ve revealed a follow-up
issue, though — `g_socket_listener_add_any_inet_port()` doesn’t try a
fallback IPv4-only socket if it tries an IPv6 socket and that socket
accepts IPv4 but then fails to `listen()`. I’ve filed issue #3604 for
that.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1250
The array was declared one byte too short to contain the trailing nul
byte for the string literal. Spotted by gcc 15.
Fix it by allowing the compiler to work out the array length.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
GUnixFDList actually comes *after* the GDBusMethodInvocation, but this
was mistakenly putting it first.
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Three of the four GApplicationCommandLine examples contained this line:
g_application_set_inactivity_timeout (app, 10000);
It is not explained (which could be confusing for readers trying to
understand the examplese), or necessary. Worse, it causes two of the
examples to pause for ten seconds if they are invoked with no command-line
arguments, which makes them seem broken (and would presumably be reported
as a bug in any real application).
So, remove these calls.
Fixes#3615
During "as-installed" testing, we should search the GIR_DIR for GIR XML,
instead of hard-coding that it is `${prefix}/share/gir-1.0`. This is
not the case on at least Debian, in order to make it possible to
install more than one architecture's flavour of `GLib-2.0.gir`,
which contains some architecture-specific `#define`s.
Also search GOBJECT_INTROSPECTION_DATADIR/GIR_SUFFIX (in practice
something like `/usr/share/gir-1.0` in all cases) to accommodate
distributions like Debian that move the architecture-independent
majority of GIR XML into /usr/share to avoid duplication, leaving
only the architecture-specific minority of files like `GLib-2.0.gir`
in the GIR_DIR.
Signed-off-by: Simon McVittie <smcv@collabora.com>
The documentation for g_spawn_async_with_pipes_and_fds() states:
> If an error occurs, child_pid, stdin_pipe_out, stdout_pipe_out, and
> stderr_pipe_out will not be filled with valid values.
Before 2dc3a6f0c80e5a8f786369eee0c45bfe19b55f4f, the `child_pid`
argument was `self->pid`, and GObject zero-initializes structs. So
the pid field was properly initialized to zero.
After 2dc3a6f0c80e5a8f786369eee0c45bfe19b55f4f, however, the out
variable is now declared inside initable_init(), and it's unitialized.
So if g_spawn_async_with_pipes_and_fds() errors out, `pid` will have
trash value in it, and the following assertion will fail:
```
g_assert (success == (pid != 0));
```
Fix that by initializing the `pid` variable to zero. Add a test to
exercise the fail code path, and prevent errors like this in the
future.
The cancelled state may be set and read by different threads, so ensure
that it's stored and managed in an atomic way.
We could in fact end up check for `g_file_monitor_is_cancelled()` in a
thread and `g_file_monitor_cancel()` or `g_file_monitor_emit_event` in
in another one.
We were reusing the same logic everywhere, while we can just reuse an
unique class to base our tests on that avoids having to copy-and-paste
code for no good reason
Add some basic support for having glib-tests-only python libraries that
can be shared across the various projects, so that we don't have to
maintain multiple copies of them.
This replaces `g_dbus_connection_register_object_with_closures()`, and
becomes the new binding-friendly version of
`g_dbus_connection_register_object()`.
The problem with `g_dbus_connection_register_object_with_closures()` is
that the `method_call_closure` kept the reference counting semantics of
`GDBusInterfaceMethodCallFunc`, in that the `invocation` argument was
`(transfer full)`, even though it was wrapped in a `GClosure`. This
couldn’t be described in introspection annotations, so the
`GDBusMethodInvocation` was being leaked by bindings. Some bindings
added workarounds to fix the leak at our direction (see
https://gitlab.gnome.org/GNOME/glib/-/issues/2600#note_1385050), which
meant we could no longer change the reference counting behaviour without
breaking those bindings (see #3559).
So let’s start afresh with
`g_dbus_connection_register_object_with_closures2()`, with correctly
defined reference counting semantics (the `GDBusMethodInvocation` is
`(transfer none)`) from the start.
Unfortunately we can’t add a `(rename-to)` annotation to the new API, as
that would effectively be an API break for existing binding code which
uses the old API via that rename.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3560
This reverts commit 092fedd5f085a2f1966b5c34befe8b603c1a0f07.
This was not the right change to make, and I shouldn’t have accepted the
MR. The situation is laid out in this comment:
https://gitlab.gnome.org/GNOME/glib/-/issues/2600#note_1385050
tl;dr: The reference on the `GDBusMethodInvocation` which is transferred
in to the `GDBusInterfaceMethodCallFunc` is balanced by a reference
transferred to `g_dbus_method_invocation_return_*()`. This is how the
refcounting has always worked for these functions, and even if we’d
probably arrange things differently if the code was written now, we
can’t change those semantics without breaking API.
In particular, bindings have various bits of custom code to account for
these reference tranfers (since they can’t be represented using
gobject-introspection annotations), so changing the semantics will break
bindings.
Fixes: #3559
Parts of the `dbus-appinfo` test need support for converting an FD to a
path, and Hurd doesn’t currently allow that (see
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4396#note_2279923).
Since there’s no fix for that visible in the medium term (new kernel
APIs will need to be added), skip parts of the `dbus-appinfo` test which
require that functionality for now.
This prevents the whole test from failing, and means we can usefully get
results from the parts of it which don’t depend on converting FDs to
paths.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3538
This could potentially eventually become a public GLib API, but there
doesn’t seem to be a huge need for it right now (e.g. this file contains
the only use of `/proc/self/fd/%d` in GLib), so let’s keep it private
for now and avoid committing to API stability just yet.
This gives time for other platforms to add their platform-specific
implementations for it too, if they need. I’ve added a couple of
pointers to what I *think* the right APIs might be, from my research,
but I have not prototyped those implementations.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
When a conversion is finished, the code would return 0 from its write
vfunc. This is disallowed by the API of g_output_stream_write() and
causes g_output_stream_splice() as used by g_converter_convert_bytes()
to turn into an infinite loop.
Instead, raise a G_IO_ERROR_MESSAGE_TOO_LARGE error so that the calling
code can decide how to deal with it.
Testcase included.
Closes#3532
This fixes commit aac56f1618aabfcf4c6b3ef1ee5b87322208e9ad — I missed
this while reviewing it, but the unit tests were partially changed to
call the new APIs, without being fully changed. This caused the build to
succeed on Linux, but fail on macOS due to using a deprecated API.
Actually, a better approach for the unit tests would be to consistently
call the *old* APIs, as they all immediately call the new APIs. Then we
get coverage of both old and new for free, at the cost of putting
`G_GNUC_BEGIN_IGNORE_DEPRECATIONS` at the top of the test file.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3492
UnixMountEntry: Deprecate g_unix_mount_* API in favor of g_unix_mount_entry_* API for GUnixMountEntry methods
Closes#3492
See merge request GNOME/glib!4337
This issue arises because the g_unix_mount_* naming convention does not match
the GUnixMountEntry instance type, confusing the introspection generator.
To resolve this, we are deprecating the g_unix_mount_* API functions that take
a GUnixMountEntry parameter and introducing equivalent g_unix_mount_entry_*
functions that correctly associate with the GUnixMountEntry instance. This change
ensures that introspection data correctly treats these as instance methods and
that documentation reflects proper ownership of returned data.
(Some minor tweaks by Philip Withnall.)
Fixes: #3492
