Re: [PATCH] erofs-utils: fix thread join loop in erofs_destroy_workqueue

赵逸凡
2026-03-18T02:15:41Z

On 3/17/2026 12:33 PM, Nithurshen wrote:
> Currently, erofs_destroy_workqueue returns immediately if a single
> pthread_join fails. This halts the teardown process, potentially
> leaving orphaned threads and leaking the workqueue's mutexes and
> worker array.
>
> Refactor the joining logic to unconditionally join all worker
> threads. Capture the first error encountered, continue joining the
> remaining threads, and ensure all workqueue resources are properly
> freed before returning the captured error.
>
> Signed-off-by: Nithurshen <nithurshen.dev@gmail.com>

Hi Nithurshe,

I'm afraid I cannot agree with this change. If pthread_join() fails, it
implies that the worker thread is still alive.

Cleaning up the synchronization primitives and the workers array at this
point would lead to a Use-After-Free issue, which is far more severe
than the current resource leak.

I believe pthread_join() seldom fails, otherwise it indicates bugs in
our codebase.

How about just leaving an error log print for this scenario? cc @hsiangkao

Thanks,

Yifan

> ---
> lib/workqueue.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/workqueue.c b/lib/workqueue.c
> index 18ee0f9..23eb460 100644
> --- a/lib/workqueue.c
> +++ b/lib/workqueue.c
> @@ -42,6 +42,8 @@ static void *worker_thread(void *arg)
>
> int erofs_destroy_workqueue(struct erofs_workqueue *wq)
> {
> + int err = 0;
> +
> if (!wq)
> return -EINVAL;
>
> @@ -53,15 +55,17 @@ int erofs_destroy_workqueue(struct erofs_workqueue *wq)
> while (wq->nworker) {
> int ret = -pthread_join(wq->workers[wq->nworker - 1], NULL);
>
> - if (ret)
> - return ret;
> + if (ret && !err)
> + err = ret;
> +
> --wq->nworker;
> }
> free(wq->workers);
> pthread_mutex_destroy(&wq->lock);
> pthread_cond_destroy(&wq->cond_empty);
> pthread_cond_destroy(&wq->cond_full);
> - return 0;
> +
> + return err;
> }
>
> int erofs_alloc_workqueue(struct erofs_workqueue *wq, unsigned int nworker,