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

2 Dec 2010


      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