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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list