We ended up always skipping showing the scheduler settings errors after
the first call, while we were already setting such variable atomically
in case it needed to.
Related to: #1672
As per the rationale explained in the previous commit, we could end up
having the unused_threads value not to be conformant to what
g_thread_pool_get_num_threads() returns, because an about-to-be-unused
thread might not be counted yet as such, while the pool threads number
has been already decreased.
To avoid such scenario, and to make sure that when all the pool's
threads are stopped, they're unmarked as unused, let's increase the
unused_threads value earlier, while we still own the pool lock so that
it will always include the pool that is not used anymore, but not yet
queued.
As per this we can update the test, not to repeat the stop-unused call
as now we're sure that when the pool has no threads anymore, the unused
threads value is also updated accordingly.
Also adding a tests with multiple pools.
In this tests we wanted to ensure that all the unused threads were
stopped, however while we were calling g_thread_pool_stop_unused_threads
some threads could still be in the process of being recycled even tough
the pool's num_thread values are 0.
In fact, stopping unused threads implies also resetting back the max
unused threads to the previous value, and in this test it caused it to
go from -1 -> 0 and back to -1, after killing the unused threads we
knew about; thus any about-to-be-unused thread that is not killed during
this call will be just left around as a waiting unused thread afterwards.
However, if this function was getting called when a thread was in
between of calling the user function and the moment it was being
recycled (and so when the pool num_threads was updated), but this thread
was not counted in unused_threads, we ended up in having a race because
all the threads were consumed from our POV, but some were actually not
yet unused, and so were kept waiting forever for some new job.
To avoid this in the test, we can ensure that we stop the unused
threads until we the number of them is really 0.
Sadly we need to repeat this as we don't have a clear point in which we
are sure about the fact that our threads are done, while it would be
wrong to stop a thread that is technically not yet marked as unused.
We could also do this in g_thread_pool_stop_unused_threads() itself, but
it would make such function to wait for threads to complete, and this is
probably not what was expected in the initial API.
Fixes: #2685
When the system supports it (as all Linux kernels ≥ 5.3 should), it’s
preferable to use `pidfd_open()` and `waitid()` to be notified of
child processes exiting or being signalled, rather than installing a
default `SIGCHLD` handler.
A default `SIGCHLD` handler is global, and can never interact well with
other code (from the application or other libraries) which also wants to
install a `SIGCHLD` handler.
This use of `pidfd_open()` is racy (the PID may be reused between
`g_child_watch_source_new()` being called and `pidfd_open()` being
called), so it doesn’t improve behaviour there. For that, we’d need
continuous use of pidfds throughout GLib, from fork/spawn time until
here. See #1866 for that.
The use of `waitid()` to get the process exit status could be expanded
in future to also work for stopped or continued processes (as per #175)
by adding `WSTOPPED | WCONTINUED` into the flags. That’s a behaviour
change which is outside the strict scope of adding pidfd support,
though.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1866Fixes: #2216
On win32, WaitForSingleObject may return before the timeout is
dispatched, as it doesn't have a resolution higher than the system tick.
Wait for ~50ms before checking the callback changes.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Do not wakeup immediately for our own context wakeup poll registration.
g_main_context_add_poll_unlocked() will call g_wakeup_signal() to wake
up the loop waiting in poll(). Doing so during context creation will
create an extra wakeup for the first poll().
Skip the wakeup call if it is the wake_up_rec registration. No other
sources/caller should ever reach that condition.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
In the case strerror_r returns an error (both in the char* variant and
in the int variant) we should not try to proceed converting the message
and adding to the errors maps, as that's likely causing errors.
So, let's just return a null string in case this happens
We had gcc-only implementations for them while both can be used in all
the supported platforms we have.
So let's just provide generic definitions, while we keep the old ones
for both consistency and retro-compatibility.
There’s a precondition assertion on the function which checks that
`group_name != NULL`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1474780
It’s failed on a CI machine
(https://gitlab.gnome.org/GNOME/glib/-/jobs/2006694) with log:
```
[unused] => pushed 100 threads onto the idle pool
[unused] stopping unused threads
[unused] waiting ONE second for threads to die
[unused] waiting ONE second for threads to die
[unused] waiting ONE second for threads to die
[unused] waiting ONE second for threads to die
[unused] waiting ONE second for threads to die
[unused] stopped idle threads, 9 remain
Bail out! GLib:ERROR:../glib/tests/thread-pool-slow.c:113:test_thread_stop_unused: assertion failed (g_thread_pool_get_num_unused_threads () == 0): (9 == 0)
```
I can’t reproduce this locally, but it’s possible that it failed because
a one second wait was not enough to enqueue all the jobs (on a heavily
loaded machine), and hence some jobs spawned thread pool threads after
calling `g_thread_pool_stop_unused_threads()`, leading to the assertion
failure.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
We have fallback in places for GNU's variadic arguments in macros, and
for static inline functions with variadic arguments as an fallback of
last resort, but going forward we are going to depend on `__VA_ARGS__`
for macros that cannot be re-implemented using a static inline function.
Fixes: #2681
Some of the CI runners disable ptrace, as they’re running inside a
container and ptrace would allow container escape. On those systems, gdb
can’t work, so skip the gdb test.
Add some additional stderr debug output to make it clear that’s the
case.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This part of test_non_utf8_printf() was missed while converting other
test expectations to look for a figure space, probably because it is
only run if a legacy ja_JP.eucjp locale exists. Debian generates a fairly
comprehensive set of locales (including some that are non-UTF-8) while
running GLib's installed-tests, so this was caught by CI.
Fixes: 7074122 "gdatetime: Pad numbers with numeric space"
Signed-off-by: Simon McVittie <smcv@debian.org>
In `g_data_remove_internal()`, call the `GDataElt:destroy` functions in the
order that they appear in `keys`, instead of the order that they are found in
`datalist`.
Fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2672
