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

[赵逸凡]

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 at 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,