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
We're calling g_object_notify so let's not make gobject to call it for
us.
This also allows to test that changing a property again doesn't lead to
dispatch properties being called.
This tests that we call a custom dispatch_properties_changed,
even in the absence of connected notify handlers. (A recent
optimization broke that and caused a regression in GTK).
When I optimized GObject to skip property notification
in some cases, I looked for whether the class has a
custom notify vfunc. I overlooked that that
dispatch_properties_changed can also be customized,
and if it is, we better not skip change notification.
This showed up as breakage in the adjustment tests
in the GTK testsuite.
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.
If a `.la` file is empty, `lt_libdir` and/or `lt_dlname` won’t be set,
but will then still be used in `g_strconcat()`, leading to invalid
output.
Detect that and return an error.
Adds a unit test.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1474756
These headers have all been written manually, by looking through the git
log for each file and noting the copyright of each significant
contribution.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
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
