GChildWatchSource uses waitpid(), among pidfd and GetExitCodeProcess().
It thus only works for child processes which the user must ensure to
exist and not being reaped yet. Also, the user must not kill() the PID
after the child process is reaped and must not race kill() against
waitpid(). Also, the user must not call waitpid()/kill() after the child
process is reaped.
Previously, GChildWatchSource would call waitpid() already when adding
the source (g_child_watch_source_new()) and from the worker thread
(dispatch_unix_signals_unlocked()). That is racy:
- if a child watcher is attached and did not yet fire, you cannot call
kill() on the PID without racing against the PID being reaped on the
worker thread. That would then lead to ESRCH or even worse, killing
the wrong process.
- if you g_source_destroy() the source that didn't fire yet, the user
doesn't know whether the PID was reaped in the background. Any
subsequent kill()/waitpid() may fail with ESRCH/ECHILD or even address
the wrong process.
The race is most visible on Unix without pidfd support, because then the
process gets reaped on the worker thread or during g_child_watch_source_new().
But it's also with Windows and pidfd, because we would have waited for
the process in g_child_watch_check(), where other callbacks could fire
between reaping the process status and emitting the source's callback.
Fix all that by calling waitpid() right before dispatching the callback.
Note that the prepare callback only has one caller, which pre-initializes
the timeout argument to -1. That may be an implementation detail and not
publicly promised, but it wouldn't make sense to do it any other way in
the caller.
Also, note that g_unix_signal_watch_prepare() and the UNIX branch of
g_child_watch_prepare() already relied on that.
Note that the variable source_timeout is already initialized upon
definition, at the beginning of the block.
It's easy to see, that no code changes the variable between the variable
definition, and the place where it was initialized. It was thus
unnecessary.
It's not about dropping the unnecessary code (the compiler could do that
just fine too). It's that there is the other branch of the "if/else", where
the variable is also not initialized. But the other branch also requires
that the variable is in fact initialized to -1, because prepare()
callbacks are free not to explicitly set the output value. So both
branches require the variable to be initialized to -1, but only one of
them did. This poses unnecessary questions about whether anything is
wrong. Avoid that by dropping the redundant code.
- if a child watch source has "using_pidfd", it is never linked in the
unix_child_watches list. Drop that check.
- replace the deep nested if, with an early "continue" in the loop,
if we detect there is nothing to do. It makes the code easier to
read.
Let's move the difference between the win/unix implementations closer to
where the difference is. Thereby, we easier see the two implementations
side by side. Splitting it at a higher layer makes the code harder to
read.
This is just a preparation for what comes next.
commit bfbe7127d5 which did a code refactor in
gobject_gdb.py introduced a bug by failing to
return the signal name when a signal had no
'detail', this was preventing pretty printing
name for signals with no 'detail'.
When `copy_file_range()` support was added, I used the definition of
`copy_file_range()` from Linux, which uses `loff_t` to abstract the
different `off*_t` types.
`loff_t` doesn’t exist on FreeBSD, so this doesn’t compile, and was
caught in subsequent asynchronous CI.
Define `loff_t` with a fallback value if it’s not defined, which should
fix this and other uses of `loff_t` in `gfile.c` (for example, if
FreeBSD ever starts declaring `splice()`).
Fixes this CI failure: https://gitlab.gnome.org/GNOME/glib/-/jobs/2812302
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
GTK lost it's '+' suffix back in 2019, according to
<https://mail.gnome.org/archives/gtk-devel-list/2019-February/msg00000.html>
This commit can be re-generated with:
git grep -l GTK+ \
| grep -v -e ^NEWS -e ^glib/tests/collate.c \
| xargs sed -i 's/GTK+/GTK/g'
Most of the changes are in comments and documentation.
Use more modern styling to the code added in the previous patch:
- split 'label: stmt; stmt;' into multiple lines
- add default: label with g_assert_not_reached() [yes, it's a bit
weird adding an assertion inside code that handles assertions, but
we should be okay since g_assertion_message_* are not public
functions and should only be used by our macros]
- use <inttypes.h> for shorter format strings
Note, however, that using uint64_t in gtestutils.h is not feasible,
since it would require adding an '#include <stdint.h>' with potential
unintended namespace pollution to older clients.
Signed-off-by: Eric Blake <eblake@redhat.com>
While x86_64 has enough precision in long double to do a round trip
from guint64 to long double and back, this is platform-specific, and
is a disservice to users trying to debug failing unit tests on other
architectures where it loses precision for g_assert_cmp{int,uint,hex}.
See also https://bugzilla.gnome.org/show_bug.cgi?id=788385 which
mentions having to add casts to specifically silence the compiler on
platforms where the precision loss occurs.
Meanwhile, g_assert_cmpuint() does an unsigned comparison, but outputs
signed values if the comparison fails, which is confusing.
Fix both issues by introducing a new g_assertion_message_cmpint()
function with a new 'u' numtype. For backwards compatibility, the
macros still call into the older g_assertion_message_cmpnum() when not
targetting 2.78, and that function still works when passed 'i' and 'x'
types even though code compiled for 2.78 and later will never invoke
it with numtype anything other than 'f'. Note that g_assert_cmpmem
can also take advantage of the new code, even though in practice,
comparison between two size_t values representing array lengths that
can actually be compiled is unlikely to have ever hit the precision
loss. The macros in signals.c test code does not have to worry about
versioning, since it is not part of the glib library proper.
Closes#2997
Signed-off-by: Eric Blake <eblake@redhat.com>
While it can’t be used in all situations, it is a little bit faster than
`splice()` in some situations, basically if the file system supports
copy on write. In other situations it’s no slower than `splice()`.
See `man copy_file_range` for the situations where it doesn’t work. In
all of these situations, it will return an error, and the GLib code will
fall through and try the existing `splice()` copy code instead.
From my testing of `time gio copy A B` with a 9GB file, the `splice()`
code path takes 22s, and the `copy_file_range()` code path takes 20s.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2863
The start of the `g_file_copy()` implementation stats the source file to
find all the attributes to copy onto the destination file, so it makes
sense to get it to store the source file size at the same time.
This saves a subsequent `stat()` call on the source FD in the btrfs
reflink or splice code. Every little helps.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
When cross-building with a non-Linux target, linux/netlink.h will not
actually be buildable, even if distributions typically put it in
/usr/include and thus exposed to the cross-compiler.
Something has changed recently which causes this error to now be emitted
when building on Windows msys2-mingw32:
```
../gio/gwin32networkmonitor.c: In function 'win_network_monitor_get_ip_info':
../gio/gwin32networkmonitor.c:92:15: error: storing the address of local variable 'prefix' in '*dest' [-Werror=dangling-pointer=]
92 | *dest = (guint8 *) &prefix.Prefix.Ipv4.sin_addr;
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
If `IP_ADDRESS_PREFIX` is defined as a scalar rather than a pointer,
that could explain the problem.
Change the function to always operate on a pointer to avoid any
potential such issues.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This should hopefully stop the kernel spending a lot of memory and disk
bandwidth creating coredumps for them unnecessarily, which slows down
the rest of the tests and generally wastes resources.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2939
This is put together through git archaeology:
```
git log -- glib/tests/assert-msg-test.c tests/assert-msg-test.c
```
The following commits were too trivial to have meaningful copyright:
- 8e59d8602c
- 44c004c84e
- 207b8cb8a5
- a1bee97d4f
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
I’ve finally found the right place in the docs to put this stuff.
This doesn’t auto-link this section from every string in the GLib
documentation, but I think that at this point (with gtk-doc in
maintenance mode, and gi-docgen not fully applied to GLib) I don’t think
we can do any better. The perfect is the enemy of the good, and having
this stuff documented somewhere means that someone can link to it from
multiple places in future *somehow*.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #116
