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

[Gao Xiang]

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

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,