
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