Some GByteArray functions already pass through the return values of
called GArray functions. Do so for all functions so callers might
react to errors and for consistency.
The Meson test harness handles that for us.
With a custom timeout, meson test -t is not useful (which is
surprising for users) and interactive debugging sessions may
terminate unexepectedly.
Make sure that arguments are not null. While internal functions are
properly protected, the code execution continues and eventually leads
to a NULL pointer dereference.
It’s possible for the server communications to finish one main context
iteration before all of the client communications, depending on how the
kernel queues socket connection messages.
Fixes CI failure: https://gitlab.gnome.org/GNOME/glib/-/jobs/5341950
```
GLib-GIO:ERROR:../gio/tests/socket-listener.c:639:test_accept_multi_simultaneously: 'clients[i].result' should not be NULL
not ok /socket-listener/accept/multi-simultaneously - GLib-GIO:ERROR:../gio/tests/socket-listener.c:639:test_accept_multi_simultaneously: 'clients[i].result' should not be NULL
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
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
The changes in commit 30ccfac9cf were not quite correct. The code is
structured so that a single reference to a `GTask` (and hence its
`AcceptSocketAsyncData` task data) is shared across the multiple
`GSocketSource`s which are created for a pending `accept_async()` call.
Setting `returned_yet` to true to short-circuit the remaining
`accept_ready()` dispatches in a given `GMainContext` iteration would
have worked, were it not for the fact that the code then immediately
dropped the last reference it had to the `GTask`, potentially freeing
the structure which contained `returned_yet`. Because of the async
nature of `GTask`, the exact timing of finalisation could vary.
This also meant that the other `GSocketSource`s were not destroyed until
an unknown time later.
Improve on that by explicitly destroying the other `GSocketSource`s as
soon as the first one returns an accepted socket. This causes
`GMainContext` to skip dispatching them, even within the same
`GMainContext` iteration. It also means the separate `returned_yet`
member is unnecessary.
This should fix the immediate issue seen in #3739. However, while
testing it I found a further issue which will be fixed in a following
commit, before I add a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #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
`GMemoryMonitor` is a singleton, which means we can’t use the usual
approach of emitting signals in the thread-default main context from the
time of construction of the object.
The next best thing is to emit them in the global default main context.
For many applications, this will be exactly what they are expecting. For
multi-threaded applications, they will need to implement their own
thread safety in the signal handler, but they would have to do that
anyway.
Currently, the signals are emitted in the GLib worker thread (for the
PSI and poll implementations of `GMemoryMonitor`) — this is the worst
option, because it means that third party signal handlers could block
the worker thread (which is precisely what the worker thread is meant to
avoid).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The memory free ratio test can be skipped when we run the test. If
proc_override is TRUE and proc path is overridden, the memory free
ratio test can be skipped.
The value overriding has to be set before testing the value and returning
the callback. Otherwise, the callback can't emit a signal to the test
program.
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>
If the first `goto out` is taken, `file` has not yet been initialised,
but `g_clear_object (&file)` is called on it, and things get unhappy.
Fix that by following standard convention and initialising
‘autoptr’-like variables at declaration time.
Spotted by scan-build in
https://gitlab.gnome.org/GNOME/glib/-/jobs/5321883.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The `mainloop` test suite takes about 775s on my machine under valgrind
with this test enabled, vs 50s without this test enabled.
This causes CI failures like
https://gitlab.gnome.org/GNOME/glib/-/jobs/5321882.
I’m not sure that valgrind will actually successfully reproduce the race
condition because it runs too slowly (but I haven’t verified that by
reverting the fix for the race).
In any case, you can still choose to run the test under valgrind with
`-m thorough`.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
This backend periodically watch the memory free ratio through sysinfo().
It signals the applications when the memory free ratio drops to 40%, 30%,
and 20% for LOW, MEDIUM, CRITICAL status, respectively.
The PSI backend is based on Kernel PSI [1]. It monitors the memory
allocation time with in a given time window. If the allocation time
is more than the given threshold, Kernel signal the application to
deliver the memory pressure events.
The current thresholds are:
LOW: 70ms out of 2 seconds for partial stall
MEDIUM: 100ms out of 2 seconds for partial stall
CRITICAL: 100ms out of 2 seconds for full stall
[1] https://docs.kernel.org/accounting/psi.html
This class provides the shared functions, such as sending a signal and
string and value conversion. The backend classes should inherit this
class to get the shared functions.
It adds a configure time check for `sysinfo()`, as some systems don’t
have it.
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>
