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
It wasn’t clear what the fallback behaviour of IPv4 vs IPv6 was, or that
the same port was used for both.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
It wasn’t clear what the fallback behaviour of IPv4 vs IPv6 was, or that
the same port was used for both.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
In add_any_inet_port() for the case where we successfully listen()ed
on IPv6, but failed to listen on IPv4, we would erroneously unref the
IPv6 socket which we just gave away ownership of by adding it to
priv->sockets.
However, given that we have a successful IPv6 socket, we shouldn’t
report it as an error if the IPv4 listen() fails. While this code tries
quite hard to return both sockets, returning only one seems to be the
best we can do in this situation.
This issue was observed in an Android 4.4 ARM emulator. It’s possible
the emulator deferred the bind() on the host until the listen() on the
client.
Based on a patch by Ole André Vadla Ravnås <oleavr@gmail.com>.
https://gitlab.gnome.org/GNOME/glib/issues/1250
Nicks and blurbs don't have any practical use for gio/gobject libraries.
Leaving tests untouched since this features is still used by other libraries.
Closes#2991
Previously these properties would have been documented using the strings
from the pspec, but those will be removed in the following commit. Re-add
the documentation using those strings, but as gi-docgen documentation
comments.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #2991
Following Emmanuele's instructions for use of introspection annotations:
https://www.bassi.io/articles/2023/02/20/bindable-api-2023/
I have audited all uses of the (closure) annotation in glib and
determined that only a handful are correct. This commit changes almost
all of our use of (closure) annotations to conform to Emmanuele's rules.
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.
This commit was entirely generated using the command:
```
git ls-files gio/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
gio/gsocketlistener.c: In function ‘g_socket_listener_close’:
gio/gsocketlistener.c:1019:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
1019 | for (i = 0; i < listener->priv->sockets->len; i++)
| ^
gio/gsocketlistener.c: In function ‘g_socket_listener_set_backlog’:
gio/gsocketlistener.c:993:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
993 | for (i = 0; i < listener->priv->sockets->len; i++)
| ^
gio/gsocketlistener.c: In function ‘add_sources’:
gio/gsocketlistener.c:612:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘guint’ {aka ‘unsigned int’}
612 | for (i = 0; i < listener->priv->sockets->len; i++)
| ^
Using the generic marshaller has drawbacks beyond performance. One such
drawback is that it breaks the stack unwinding from the Linux kernel due
to having unsufficient data to walk past ffi_call_unixt64. That means that
performance profiling by application developers looks grouped among
seemingly unrelated code paths.
While we can't fix the kernel unwinding here, we can provide proper
c_marshallers and va_marshallers for objects within Gio so that
performance profiling of applications is more reliable.
Related to GNOME/Initiatives#10
When calling g_socket_listener_accept_socket_async() on a
GSocketListener with multiple sockets, the accept_ready() callback is
called for the first incoming connection on each socket. It will return
success/failure for the entire accept_socket_async() GTask, and then
free the GSources for listening for incoming connections on the other
sockets in the GSocketListener. The GSources are freed when the GTask is
finalised.
However, if incoming connections arrive for multiple sockets within the
same GMainContext iteration, accept_ready() will be called multiple
times, and will call g_task_return_*() multiple times, before the GTask
is finalised. Calling g_task_return_*() multiple times is not allowed.
Propagate the first success/failure, as before, but then ignore all
subsequent incoming connections until the GTask is finalised.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
GSocketListener can keep internal references to itself for pending
accept() calls, which mean that it can stay alive (and keep listening
on ports) even after a user drops their last reference to it. They need
to call g_socket_listener_close() explicitly to avoid that.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://bugzilla.gnome.org/show_bug.cgi?id=794207
There are a few places where commit 18a33f72 replaced valid (nullable)
(optional) annotations with just (optional). That has a different
meaning.
(nullable) (optional) can only be applied to gpointer* parameters, and
means that both the gpointer* and returned gpointer can be NULL. i.e.
The caller can pass in NULL to ignore the return value; and the returned
value can be NULL.
(optional) can be applied to anything* parameters, and means that the
anything* can be NULL. i.e. The caller can pass in NULL to ignore the
return value. The return value cannot be NULL.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.
It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
This allows the caller to know when a socket has been bound so that
it can for instance set the SO_SENDBUF and SO_RECVBUF socket options
before listen is called
https://bugzilla.gnome.org/show_bug.cgi?id=738207
Instead of closing the sockets explicitly, let them close themselves
when their final reference is dropped. This makes use of
g_socket_listener_add_socket() more natural.
https://bugzilla.gnome.org/show_bug.cgi?id=732107
As it turns out, we have examples of internal functions called
type_name_get_private() in the wild (especially among older libraries),
so we need to use a name for the per-instance private data getter
function that hopefully won't conflict with anything.
GFile allows for the possibility that external implementations may not
support thread-default contexts yet, via
g_file_supports_thread_contexts(). GVolumeMonitor is not yet
thread-default-context aware.
Add a test program to verify that basic gio async ops work correctly
in non-default contexts.
http://bugzilla.gnome.org/show_bug.cgi?id=579984
Currently, to implement cancellability correctly, all synchronous
calls to GSocket must be preceded by a g_socket_condition_wait() call,
(even though GSocket does this internally as well) and all
asynchronous calls must do occasional manual
g_cancellable_is_cancelled() checks. Since it's trivial to do these
checks inside GSocket instead, and we don't particularly want to
encourage people to use the APIs non-cancellably, move the
cancellation support into GSocket and simplify the existing callers.
http://bugzilla.gnome.org/show_bug.cgi?id=586797
