On 26.05.2026 22:33, Peter Xu wrote:
On Thu, May 21, 2026 at 09:06:07PM +0200, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <[email protected]>
thread_pool_adjust_max_threads_to_work() is supposed to give each task its
own thread by setting the pool max thread count limit accordingly.
However, if there aren't any tasks currently in the pool the pool max
thread count will be set to 0, which will trigger an assertion failure
in thread_pool_set_max_threads() - because setting this value would
completely block the pool by not allowing it to process any submitted
tasks.
This also can happen if a task is submitted via
thread_pool_submit_immediate() to an empty pool but the task completes so
quickly that by the time this function calls
thread_pool_adjust_max_threads_to_work() the pool again has no unfinished
tasks in it.
Sorry for a late comment. Just curious: how easy is this to reproduce?
It's difficult to reproduce in most setups.
My main VFIO live migration setup never hit it for more than a year, other
similar setup hit it recently 3 times.
On the other hand, putting sleep(5) in the middle of
thread_pool_submit_immediate() makes it reproduce nearly always for me.
Fix this by making sure that the pool is allowed to create at least 1
thread.
But then it means we have no work and then we will create one thread does
nothing..
thread_pool_adjust_max_threads_to_work() is currently called only from
thread_pool_submit_immediate().
If the API user truly wants no threads in the pool for time being
(even though this will completely block the pool) they can use
thread_pool_submit() to submit their task(s).
This won't call thread_pool_adjust_max_threads_to_work() by itself.
Also, since the thread pool is created with zero initial threads by
default the patch does *not* mean that the each newly created pool
will have one idle thread now.
I suspect the real culprit is we released the cur_work_lock during the
whole process of thread_pool_submit_immediate(). If we take it during the
whole window this will be a no-issue too.
True, however this will need splitting both thread_pool_submit() and
thread_pool_adjust_max_threads_to_work() into two versions each: one
which takes pool->cur_work_lock, another which does not and then
take pool->cur_work_lock on their behalf in thread_pool_submit_immediate().
Not to mention this would also mean putting the whole g_thread_pool_push()
-> g_thread_pool_start_thread() machinery under pool->cur_work_lock in this
case instead of just pool->cur_work increment.
I think doing this such way would add unnecessary complexity considering
that its alternative of this patch is a single-line trivial change.
The other question is, if it is awkward to manually adjust num of threads,
shall we set num to be -1 (unlimited) while pool created?
We can't - Glib thread pool API does not allow unlimited exclusive pools.
Thanks,
Thanks,
Maciej
