[libvirt] [PATCH v4 RESEND 1/4] threadpool impl

[Hu Tao]

On Thu, Dec 02, 2010 at 12:28:17PM +0000, Daniel P. Berrange wrote:
> On Thu, Dec 02, 2010 at 03:26:57PM +0800, Hu Tao wrote:
> > +virThreadPoolPtr virThreadPoolNew(size_t minWorkers,
> > +                                  size_t maxWorkers,
> > +                                  virThreadPoolJobFunc func,
> > +                                  void *opaque)
> > +{
> > +    virThreadPoolPtr pool;
> > +    size_t i;
> > +
> > +    if (minWorkers > maxWorkers)
> > +        minWorkers = maxWorkers;
> > +
> > +    if (VIR_ALLOC(pool) < 0) {
> > +        virReportOOMError();
> > +        return NULL;
> > +    }
> > +
> > +    pool->jobList.head = NULL;
> > +    pool->jobList.tail = &pool->jobList.head;
> > +
> > +    pool->jobFunc = func;
> > +    pool->jobOpaque = opaque;
> > +
> > +    if (virMutexInit(&pool->mutex) < 0)
> > +        goto error;
> > +    if (virCondInit(&pool->cond) < 0)
> > +        goto error;
> > +    if (virCondInit(&pool->quit_cond) < 0)
> > +        goto error;
> > +
> > +    if (VIR_ALLOC_N(pool->workers, minWorkers) < 0)
> > +        goto error;
> > +
> > +    pool->maxWorkers = maxWorkers;
> > +    for (i = 0; i < minWorkers; i++) {
> > +        if (virThreadCreate(&pool->workers[i],
> > +                            true,
> > +                            virThreadPoolWorker,
> > +                            pool) < 0) {
> > +            virThreadPoolFree(pool);
> > +            return NULL;
> > +        }
> > +        pool->nWorkers++;
> > +    }
> > +
> > +    return pool;
> > +
> > +error:
> > +    VIR_FREE(pool->workers);
> > +    ignore_value(virCondDestroy(&pool->quit_cond));
> > +    ignore_value(virCondDestroy(&pool->cond));
> > +    virMutexDestroy(&pool->mutex);
> > +    return NULL;
> > +}
> 
> This is leaking 'pool' on error. IMHO it is preferrable to
> just call virThreadPoolDestroy, otherwise anytime we add
> another field to virThreadPoolPtr struct, we have to consider
> updating 2 cleanup paths.

Agree. Since it is in error clean path (thus thread pool is not yet
created) it doesn't matter to lock on a broken mutex or wait on a
broken cond.

-- 
Thanks,
Hu Tao