When unref'ing child sources, the lock is already held. But instead of
passing TRUE to g_source_unref_internal it currently passes whether the
lock was already held outside of the current invocation. Just pass TRUE
to fix this possible issue.
By default, when using g_subprocess_launcher_take_fd() to pass an
FD to a child, the GSubprocessLauncher object also takes ownership
of the FD in the parent, and closes it during finalize(). This is
a reasonable assumption in the majority of the cases, but sometimes
it isn't a good idea.
An example is when creating a GSubprocessLauncher in JavaScript:
here, the destruction process is managed by the Garbage Collector,
which means that those sockets will remain opened for some time
after all the references to the object has been droped. This means
that it could be not possible to detect when the child has closed
that same FD, because in order to make that work, both FDs
instances (the one in the parent and the one in the children) must
be closed. This can be a problem in, as an example, a process that
launches a child that communicates with Wayland using an specific
socket (like when using the new API MetaWaylandClient).
Of course, it isn't a valid solution to manually call close() in
the parent process just after the call to spawn(), because the FD
number could be reused in the time between it is manually closed,
and when the object is destroyed and closes again that FD. If that
happens, it will close an incorrect FD.
One solution could be to call run_dispose() from Javascript on the
GSubprocessLauncher object, to force freeing the resources.
Unfortunately, the current code frees them in the finalize()
method, not in dispose() (this is fixed in !1670 (merged) ) but it
isn't a very elegant solution.
This proposal adds a new method, g_subprocess_launcher_close(),
that allows to close the FDs passed to the child. To avoid problems,
after closing an FD with this method, no more spawns are allowed.
Fix: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1677
It’s landed in kernel 5.9: http://lkml.iu.edu/hypermail/linux/kernel/2008.0/02649.html
Note, this is untested because I currently don’t have kernel 5.9. We can
fix anything up if it breaks once the new syscall is wrapped in glibc.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Originally, GSocketClient returned whatever error occured last. Turns
out this doesn't work well in practice. Consider the following case:
DNS returns an IPv4 and IPv6 address. First we'll connect() to the
IPv4 address, and say that succeeds, but TLS is enabled and the TLS
handshake fails. Then we try the IPv6 address and receive ENETUNREACH
because IPv6 isn't supported. We wind up returning NETWORK_UNREACHABLE
even though the address can be pinged and a TLS error would be more
appropriate. So instead, we now try to return the error corresponding
to the latest attempted GSocketClientEvent in the connection process.
TLS errors take precedence over proxy errors, which take precedence
over connect() errors, which take precedence over DNS errors.
In writing this commit, I made several mistakes that were caught by
proxy-test.c, which tests using GSocketClient to make a proxy
connection. So although adding a new test to ensure we get the
best-possible error would be awkward, at least we have some test
coverage for the code that helped avoid introducing bugs.
Fixes#2211
We should never return unknown errors to the application. This would be
a glib bug.
I don't think it's currently possible to hit these cases, so asserts
should be OK. For this to happen, either (a) a GSocketAddressEnumerator
would have to return NULL on its first enumeration, without returning an
error, or (b) there would have to be a bug in our GSocketClient logic.
Either way, if such a bug were to exist, it would be better to surface
it rather than hide it.
These changes are actually going to be effectively undone in a
subsequent commit, as I'm refactoring the error handling, but the commit
history is a bit nicer with two separate commits, so let's go with two.
GSocketAddressEnumerator encapsulates the details of how DNS happens, so
we don't have to think about it. But we may have taken encapsulation a
bit too far, here. Usually, we resolve a domain name to a list of IPv4
and IPv6 addresses. Then we go through each address in the list and try
to connect to it. Name resolution happens exactly once, at the start.
It doesn't happen each time we enumerate the enumerator. In theory, it
*could*, because we've designed these APIs to be agnostic of underlying
implementation details like DNS and network protocols. But in practice,
we know that's not really what's happening. It's weird to say that we
are RESOLVING what we know to be the same name multiple times. Behind
the scenes, we're not doing that.
This also fixes#1994, where enumeration can end with a RESOLVING event,
even though this is supposed to be the first event rather than the last.
I thought this would be hard to fix, even requiring new public API in
GSocketAddressEnumerator to peek ahead to see if the next enumeration is
going to return NULL. Then I decided we should just fake it: always emit
both RESOLVING and RESOLVED at the same time right after each
enumeration. Finally, I realized we can emit them at the correct time if
we simply assume resolving only happens the first time. This seems like
the most elegant of the possible solutions.
Now, this is a behavior change, and arguably an API break, but it should
align better with reasonable expectations of how GSocketClientEvent
ought to work. I don't expect it to break anything besides tests that
check which order GSocketClientEvent events are emitted in. (Currently,
libsoup has such tests, which will need to be updated.) Ideally we would
have GLib-level tests as well, but in a concession to pragmatism, it's a
lot easier to keep network tests in libsoup.
This isn't an API guarantee, but it's a potentially-surprising
behavior difference between the sync and async functions that is good
to know about, especially because our sync and async functions are
normally identical.
The linux kernel does not know that the socket will be used
for connect or listen and if you bind() to a local address it must
reserve a random port (if port == 0) at bind() time, making very easy
to exhaust the ~32k port range, setting IP_BIND_ADDRESS_NO_PORT tells
the kernel to choose random port at connect() time instead, when the
full 4-tuple is known.
This allows programs that want to change how log messages are printed,
such as gnome-terminal (gnome-terminal#42) and Flatpak, to override
the log-writer or the legacy log-handler without having to reimplement
the G_MESSAGES_DEBUG filtering logic.
Signed-off-by: Simon McVittie <smcv@collabora.com>
GLib code normally prints info and debug messages to stdout,
but that interferes with programs that are documented to produce
machine-readable output such as JSON or XML on stdout. In particular,
if such a program uses a GLib-based library, setting G_MESSAGES_DEBUG
will typically result in that library's debug messages going to the
program's stdout and corrupting the machine-readable output.
Unix programs can avoid this by using dup2() to move the original stdout
to another fd, then dup2() again to make the new stdout a copy of stderr,
but it's easier if we provide a way to not write debug messages to
stdout in the first place. Calling
g_log_writer_default_set_use_stderr (TRUE) results in behaviour
resembling Python's logging.basicConfig(), with all diagnostics going
to stderr.
Suggested by Allison Karlitskaya on glib#2087.
Signed-off-by: Simon McVittie <smcv@collabora.com>
"lower bound" and "upper bound" operations have been recently added to
GTree.
Let's add some tests for them where other GTree tests live.
Since adding keys in-order doesn't exercise the GTree insertion code very
well let's make sure they are inserted in a random order instead.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
The basic API that this commit adds allows in-order iterating over a GTree.
For this the following API were implemented or exported:
1) Returning the first or the last node in the tree,
2) Taking a pointer to a node in the tree and returning the previous or the
next in-order node,
3) Allowing to do a binary search for a particular key value and returning
the pointer to its node,
4) Returning the newly inserted or set node from both insert and replace
functions, so this node is immediately available and does not have to be
looked up,
5) Traversing the tree in-order providing a node pointer to the
caller-provided traversal function.
Most of the above functions were already present in the code, but they
returned the value that is stored at a particular node instead of the
pointer to the node itself.
So most of the code for these new API calls is shared with these existing
ones, just adapted to return the pointer to the node.
Additionally, the so called "lower bound" and "upper bound" operations
were implemented.
The first one returns the first element that is greater than or equal to
the searched key, while the second returns the first element that is
strictly greater than the searched key.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
* Add a debug assert checking whether we have really removed all nodes in
g_tree_remove_all(),
* Print a "LEFT" and "RIGHT" headers before printing a particular tree
branch in g_tree_node_dump(),
* Make the whole thing actually buildable again in the debug mode by
conditionally providing g_tree_dump() declaration in glib/gtree.h.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
`g_local_file_fstatat()` needs to fall back to returning an error if
`fstatat()` isn’t defined, which is the case on older versions of macOS
(as well as Windows, which was already handled). Callers shouldn’t call
`g_local_file_fstatat()` in these cases. (That’s already the case.)
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2203
Expose a function that prepares an attribute query string to be passed
to g_file_query_info() to get a list of attributes normally copied with
the file. This function is used by the implementation of
g_file_copy_attributes, and it's useful if one needs to split
g_file_copy_attributes into two stages, for example, when nautilus does
a recursive move of a directory. When files are moved from the source
directory, its modification time changes. To preserve the mtime on the
destination directory, it has to be queried before moving files and set
after doing it, hence these two stages.
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
The return value from `g_utf8_get_char_validated()` is a `gunichar`,
which is unsigned, so comparing it with `> 0` is always going to return
true, even for return values `(gunichar) -1` and `(gunichar) -2`, which
indicate errors.
Handle them more explicitly.
oss-fuzz#26083
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This is very unlikely to happen, but add error handling to mirror the
other calls to `safe_open()`, and shut Coverity up.
Coverity CID: #1430611
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
The GSubprocessLauncher class lacks a dispose() method, and frees
all their resources in the finalize() method.
This is a problem with Javascript because the sockets passed to a
child process using g_subprocess_launcher_take_fd() aren't closed
in the parent space until the object is fully freed. This means
that if the child closes a socket, it won't be detected until the
GSubprocessLauncher object has been freed by the garbage
collector.
Just closing the socket externally is not a valid solution,
because the finalize() method will close it again, and since
another file/pipe/socket could have been opened in the meantime
and use the same FD number, the finalize() method would close
an incorrect FD.
An example is launching a child process that uses its own
socket for Wayland: the parent creates two sockets with
socketpair(), passes one to the Wayland API (wl_client_create()),
and the other is passed to the child process using
g_subprocess_launcher_take_fd(). But now there are two instances
of that second socket: one in the parent, and another one in the
child process. That means that, if the child closes its socket (or
dies), the Wayland server will not detect that until the
GSubprocessLauncher object is fully destroyed. That means that a
GSubprocessLauncher created in Javascript will last for several
seconds after the child dies, and every window or graphical element
will remain in the screen until the Garbage Collector destroys the
GSubprocessLauncher object.
This patch fixes this by moving the resource free code into a
dispose() method, which can be called from Javascript. This allows
to ensure that any socket passed to the child with
g_subprocess_launcher_take_fd() can be closed even from Javascript
just by calling the method run_dispose().
Fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1670
If this fails to compile on some particularly bizarre Unix platform,
we can relax these assertions; but our expectation is that gssize is
POSIX ssize_t, and that on Unix, GPid is POSIX pid_t.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This is a step towards glib#1484. We officially require a C99 toolchain,
so we can statically assert that our artisanal hand-crafted integer
types are compatible with the ones we would like to recommend people
use instead.
If there are *still* platforms where <stdint.h> is problematic, these
static assertions can act as an early-warning that future GLib releases
will make a C99-compliant <stdint.h> a hard requirement, in ways that
are less straightforward to avoid (see glib#1484 and glib!1300).
Signed-off-by: Simon McVittie <smcv@collabora.com>
We have been passing a &resolved_identifier address around for multiple
functions to set it. Each function may either:
1. leaving it for the next function to set, if returning early;
2. set it to a duplicate of the passed identifier, if not NULL;
3. get a fallback value and set it, otherwise.
This can be simplified by setting it early to either:
1. a duplicate of the passed identifier, if not NULL;
2. a fallback value, otherwise.
This way we can avoid some unnecessary string duplication and freeing.
Also, on Windows, we avoid calling windows_default_tzname() twice.
But the main motivation for this change is enabling the performance
optimization in the next commit.
When the TZ environment variable is not set, we get the local timezone
identifier by reading specific files.
We are going to need these identifiers earlier, so split this logic into
its own function, in preparation for the next commit.
Based on idea proposed by Sebastian Keller <skeller@gnome.org>.
