thread-pool: signal requests while locked

thread_pool_free() might have been called on the `pool`, which would
be a reason for worker_thread() to quit. In this case,
`pool->request_cond` is been destroyed.

If worker_thread() didn't managed to signal `request_cond` before it
been destroyed by thread_pool_free(), we got:
    util/qemu-thread-posix.c:198: qemu_cond_signal: Assertion `cond->initialized' failed.

Reported here:
    https://lore.kernel.org/all/ZJwoK50FcnTSfFZ8@MacBook-Air-de-Roger.local/T/#u

Fixes: 900fa208f506 ("thread-pool: replace semaphore with condition variable")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

There's maybe an issue in thread_pool_submit_aio() as well, but maybe
it's much less likely to happen or should never happen?.

diff --git a/util/thread-pool.c b/util/thread-pool.c
index 0d97888df01a118761e35da9f07722eb3a53ad3c..d1c1652e87649ba95eda8b1da42a8eb4ab0c90fe 100644 (file)
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -120,13 +120,14 @@ static void *worker_thread(void *opaque)
 
     pool->cur_threads--;
     qemu_cond_signal(&pool->worker_stopped);
-    qemu_mutex_unlock(&pool->lock);
 
     /*
      * Wake up another thread, in case we got a wakeup but decided
      * to exit due to pool->cur_threads > pool->max_threads.
      */
-    qemu_cond_signal(&pool->request_cond);
+    qemu_cond_signal(&pool->request_cond); // We probably need lock on &pool->lock
+    // without a lock, thread_pool_free() could destroy the condition.
+    qemu_mutex_unlock(&pool->lock);
     return NULL;
 }
 
@@ -267,7 +268,7 @@ BlockAIOCB *thread_pool_submit_aio(ThreadPoolFunc *func, void *arg,
     }
     QTAILQ_INSERT_TAIL(&pool->request_list, req, reqs);
     qemu_mutex_unlock(&pool->lock);
-    qemu_cond_signal(&pool->request_cond);
+    qemu_cond_signal(&pool->request_cond); // do we need a lock here as well?
     return &req->common;
 }