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

Gao Xiang
2026-03-18T03:10:36Z

On 2026/3/18 10:15, 赵逸凡 wrote:
> 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.

Yes, I wonder the real error number here, it seems there
is another bug somewhere.

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

Agreed, we should print the error message for each failure
instead.

Thanks,
Gao Xiang

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