g_module_symbol() internally calls CreateToolhelp32Snapshot() which
in some circumstances returns ERROR_BAD_LENGTH and according to the docs
should lead to CreateToolhelp32Snapshot() being retried by the caller.
This retry logic was missing and for example led to g_module_symbol()
not succeeding if another thread happened to call the wrong function
at the wrong time.
This got noticed in the g-i build of gtk4 where g-i would call g_module_symbol()
on all gtk4 _get_type symbols and while inspecting the properties gtk4 would
spawn a thread calling SHGetSpecialFolderLocation() somewhere down the line.
During the call to SHGetSpecialFolderLocation() CreateToolhelp32Snapshot() would
return with ERROR_BAD_LENGTH for a short period of time and make g_module_symbol()
fail, which lead to "Invalid GType function" errors in g-i.
Fixes https://gitlab.gnome.org/GNOME/gtk/-/issues/3213
Symbols on x86 are prefixed with an underscore
but symbols on x64/ARM/ARM64 are not.
Relevant information concerning the prefixes for the architectures
can be found in the vcpkg project [1,2], where arm and arm64 builds
are part of the CI.
Specifically, _WIN64 is defined on Windows ARM64, so this issue is
only visible when building on ARM32.
[1] 08e74979df/ports/glib/fix-arm-builds.patch
[2] https://github.com/microsoft/vcpkg/pull/6116
This doesn't trigger the cancellation assertion issue when run locally
(the task didn't return yet, so the error is simply overwritten), but
perhaps it ever does in CI. Anyhow, it's good to have a cancellation
test.
After a splice operation is finished, it attempts to 1) close input/output
streams, as per the given flags, and 2) return the operation result (maybe
an error, too).
However, if the operation gets cancelled early and the streams indirectly
closed, the splice operation will try to close both descriptors and return
on the task when both are already closed. The catch here is that getting the
streams closed under its feet is possible, so the completion callback would
find both streams closed after returning on the first close operation and
return the error, but then the second operation could be able to trigger
a second error which would be returned as well.
What happens here is up to further race conditions, if the task didn't
return yet, the returned error will be simply replaced (but the old one not
freed...), if it did already return, it'll result in:
GLib-GIO-FATAL-CRITICAL: g_task_return_error: assertion '!task->ever_returned' failed
Fix this by flagging the close_async() callbacks, and checking that both
close operations did return, instead of checking that both streams are
closed by who knows.
This error triggers a semi-frequent CI failure in tracker, see the summary at
https://gitlab.gnome.org/GNOME/tracker/-/issues/240
Behaviour in that case is implementation-defined and how many bytes were
actually written can't be expressed by the return value anymore.
Instead do a short write of G_MAXSSIZE bytes and let the existing loop
for handling short writes takes care of the remaining length.
glib/gfileutils.c: In function ‘write_to_file’:
glib/gfileutils.c:1176:19: error: comparison of integer expressions of different signedness: ‘gssize’ {aka ‘long int’} and ‘gsize’ {aka ‘long unsigned int’} [-Werror=sign-compare]
1176 | g_assert (s <= length);
| ^~
glib/gmacros.h:939:25: note: in definition of macro ‘G_LIKELY’
939 | #define G_LIKELY(expr) (expr)
| ^~~~
glib/gfileutils.c:1176:7: note: in expansion of macro ‘g_assert’
1176 | g_assert (s <= length);
| ^~~~~~~~
Related to issue #1735 (Get back to a -werror build)
It turns out that our async write operation implementation is broken
on non-O_NONBLOCK pipes, because the default async write
implementation calls write() after poll() said there were some
space. However, the semantics of pipes is that unless O_NONBLOCK is set
then the write *will* block if the passed in write count is larger than
the available space.
This caused a deadlock in https://gitlab.gnome.org/GNOME/glib/-/issues/2182
due to the loop-back of the app stdout to the parent, but even without such
a deadlock it is a problem that we may block the mainloop at all.
In the particular case of g_subprocess_communicate() we have full
control of the pipes after starting the app, so it is safe to enable
O_NONBLOCK (i.e. we can ensure all the code using the fd after this can handle
non-blocking mode).
This fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2182
When an app is spawned using g_desktop_app_info_launch_uris_with_spawn
it will expand the various token in the app's commandline with the
URIs of the files to open. The expand_macro() function that is used for
this advances the pointer to the URI list to show up to which entries
it used.
To not loose the pointer to the list head a duplicate of the URI list
was actually passed to expand_macro(). However, it's not necessary to
create a copy of the URI list for that as expand_macro() will only
change which element the pointer will point to.
This behaviour actually caused the duplicated list to be leaked as the
the list pointer is NULL once all URIs are used up by expand_macro()
and thus nothing was freed at the end of the function.
Allocate a working buffer before calling `fork()` to avoid calling
`malloc()` in the async-signal-safe context between `fork()` and
`exec()`, where it’s not safe to use.
In this case, the buffer is used to assemble a wrapper around `argv` so
it can be run under `/bin/sh`.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2140
Allocate a working buffer before calling `fork()` to avoid calling
`malloc()` in the async-signal-safe context between `fork()` and
`exec()`, where it’s not safe to use.
In this case, the buffer is used to assemble elements from `PATH` with
the binary from `argv[0]` to try executing them.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
Query the environment before calling `fork()` so that it doesn’t have to
be called in the async-signal-safe context between `fork()` and
`exec()`.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
They’re not safe to call in an async-signal-safe context on Linux.
`sysconf()` is safe to call on FreeBSD and OpenBSD (at least), so
continue doing that.
This will reduce performance in the (already low performance) fallback
case where `/proc` is inaccessible to a forked process on Linux, while
spawning a subprocess.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
Use the error handling infrastructure which already exists for other
failures in the async-signal-safe context.
`g_assert()` is unlikely to have caused problems in practice because it
is only async-signal-unsafe when the assertion condition fails.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
While `g_ascii_isdigit()` *is* currently async-signal-safe, it’s going
to be hard to remember to keep it that way if the implementation changes
in future.
It seems more robust to just reimplement it here, given that it’s not
much code.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
Use normal `close()` instead, which is guaranteed to be
async-signal-safe.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
Functions called between `fork()` and `exec()` have to be
async-signal-safe.
Add a comment to each function which is called in that context, and
`FIXME` comments to the non-async-signal-safe functions which end up
being called as leaves of the call graph.
The following commits will fix those `FIXME`s.
See `man 7 signal-safety`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2140
While these assertions look right at the first glance,
they actually crash the program. That's because GObject
insists on initializing all construct-only properties
to their default values, which results in
g_win32_registry_key_set_property() being called multiple
times with NULL string, once for each unset property.
If "path" is actually set by the caller, a subsequent
call to set "path-utf16" to NULL will fail an assertion,
since absolute_path is already non-NULL.
With assertions moved the set-to-NULL calls bail out before
an assertion is made.
Have the generated .c code decorate the prototypes with "G_MODULE_EXPORT"
instead of "extern" when --internal is not being used, so that we also
export the symbols from the generated code on Visual Studio-style
compilers. If --internal is used, we decorate the prototypes with
"G_GNUC_INTERNAL", as we did before.
Note that since the generated .c code does not attempt to include the
generated headers (if one is also generated), the gnerated headers are
still generated as they were before.
When timeout grater than 0 in g_poll function, the WaitForMultipleObjects
call will wait for all the threads to return, but when only one thread
got an event the others will sleep until the timeout elapses, and causes
a stall. Triggering the stop event in g_poll in this case is useless as
it is triggered when all the threads where already signaled or timed-out.
Closes: https://gitlab.gnome.org/GNOME/glib/issues/2107
