As the new comments in the code try to explain, this fixes infinite
blocking which could happen when calling
`g_socket_listener_accept_async()` multiple times in parallel, with more
parallel calls than there are pending incoming connections on any of the
`GSocket`s in the `GSocketListener`.
The way `g_socket_listener_accept_async()` works is to create a set of
`GSocketSource`s when it’s called, one for each of the `GSocket`s in the
`GSocketListener`. Those sources are attached to the main context,
polling for `G_IO_IN` (indicating that the socket has a pending incoming
connection to accept).
When one of the socket sources polls ready, `g_socket_accept()` is
called on it, and a new connection is created.
If there are multiple pending `g_socket_listener_accept_async()` calls,
there are correspondingly multiple `GSocketSource` sources for each
`GSocket` in the `GSocketListener`. They will all poll ready in a single
`GMainContext` iteration. The first one to be dispatched will
successfully call `g_socket_accept()`, and subsequent ones to dispatch
will do likewise until there are no more pending incoming connections.
At that point, any remaining socket sources polling ready in that
`GMainContext` iteration will call `g_socket_accept()` on a socket which
is *not* ready to accept, and that will block indefinitely, because
`GSocket` has its own blocking layer on top of `poll()`.
This is not great.
It seems like a better approach would be to disable `GSocket`’s blocking
code, because `GSocketListener` is using `poll()` directly. We only need
one source of poll truth. So, do that.
Unfortunately, that’s complicated by the fact that
`g_socket_listener_add_socket()` allows third party code to provide its
own `GSocket`s to listen on. We probably can’t unilaterally change those
to non-blocking mode, so users of that API will get what they ask for.
That might include blocking indefinitely. I’ve adjusted the
documentation to mention that, at least.
The changes are fairly simple; the accompanying unit test is less
simple. Shrug. It tests for the scenario fixed by this commit, plus the
scenario fixed by the previous commit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3739
As with the previous commit, but for `g_socket_connect()`, which is the
other cancellable use of `g_socket_condition_wait()` in the file.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Currently, if `g_socket_accept()` is called with a cancelled cancellable
and the socket is in non-blocking mode, `G_IO_ERROR_CANCELLED` is not
returned, because the cancellable is only checked in the call to
`g_socket_condition_wait()`, which only happens in blocking mode.
Fix that and add a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3739
It might be possible for the `low-memory-warning` signal to be emitted
(by the GLib worker thread) before the test has connected to it, which
could cause the tests to loop forever.
Potentially fixes
https://gitlab.gnome.org/pwithnall/glib/-/jobs/5322491, though I have
not been able to reproduce the race locally.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This will catch regressions like
fc030b2b64 if they happen again in future,
by testing that fallback argument parsing code path in
`g_application_run()`.
Heavily based on the PyGObject `test_local_and_remote_command_line` unit
test at
578a55982a/tests/test_gio.py (L289).
Thanks to Arjan Molenaar for investigating the failure and writing it
up in !4703.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The dbus-appinfo test was asserting that before_emit only happens
when we haven't seen a startup ID yet. But the docs imply that it
gets emitted for every activate/open/commandline, which may well
happen repeatedly. So drop this assertion.
Tests currently fail under macOS because the tool claims not to work
on apple devices. Since I cannot disprove this myself, I'm disabling the
tests on Darwin.
This introduces an integration test that executes gio launch from a
variety of working directories, and checks that %k is expanded to a
location that makes sense in the context of the executed program, i.e.
an absolute path.
`g_file_enumerator_finalize` should not call `g_file_enumerator_close`
because object parts (i.e. subclass variable and/or resources) might
already be freed, causing memory safety issues.
A better place to call `g_file_enumerator_close` is
`g_file_enumerator_dispose` because it is safe to access the object
memory here.
Fixes#3713
Signed-off-by: fbrouille <150549-fbrouille@users.noreply.gitlab.gnome.org>
Each enumerated address can be of a different family type, so you have to make a socket after you have that information.
As this is just a test client we make a new socket each time for simplicity.
This works around a Meson bug
(https://github.com/mesonbuild/meson/issues/4668).
If we have a Python test which spawns a built native binary, that binary is
listed in the `depends` argument of the `test()`. On Linux, this results in
the directories containing the built libraries which the binary depends on
being added to the `LD_LIBRARY_PATH` of the test invocation. On Windows,
however, Meson currently doesn’t add those directories to `PATH` (which is
the equivalent of `LD_LIBRARY_PATH`), so we have to do it manually.
This takes the same approach as Christoph Reiter did in
gobject-introspection
(13e8c7ff80/tests/meson.build (L2)).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Unlike the previous commit, there is no `g_fdopen()` wrapper where we
can add cross-platform support for this.
I’m not adding that now just for `O_CLOEXEC` support for these two
calls, so pass the flag locally for now.
If someone wanted to add a `g_fdopen()` wrapper in future, the
`GLIB_FD_CLOEXEC` here could be refactored easily.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This adds cross-platform support for it: on glibc, musl and BSD’s libc,
the flag is natively supported. On Windows, convert it to the `N` flag,
which similarly indicates that an open file shouldn’t be inherited by
child processes.
This allows us to unconditionally pass `e` to `g_fopen()` so `O_CLOEXEC`
can easily be set on its FDs.
Also do the same for `g_freopen()`, since it shares the same underlying
mode handling code.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Some apps names or keywords contain multiple words. For example 'LibreOffice
Calc' contains the word 'Calc'. This is rightfully detected as a prefix match,
however generally it is expected that searching for 'calc' would consistantly
return 'Calculator' in first position, instead of ranking them equally.
We now prioritise tokens that would otherwise rank equal based on where they
occur in the string, giving earlier occurences precedence.
The use case for exposing this field is GTK wanting reproducible
encoding output across different OSes.
I decided to expose the OS as an integer because zlib uses an int
in its header and does not make its OS codes available as a custom
type in its API.
I also decided to limit it to values between 0 and 255 because zlib
encodes the OS as a single byte.
Test included.
Fixes: #3663
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>
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>
