Re: [libvirt] [PATCH] Threadpool: Initialize new dynamic workers

On Wed, Sep 07, 2011 at 14:11:14 +0200, Michal Privoznik wrote: > Although we were initializing worker threads during pool creating, > we missed this during virThreadPoolSendJob. This bug led to segmenation > fault as worker thread free() given argument. > --- > src/util/threadpool.c | 13 ++++++++++++- > 1 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/src/util/threadpool.c b/src/util/threadpool.c > index 70a75c0..6210b00 100644 > --- a/src/util/threadpool.c > +++ b/src/util/threadpool.c > @@ -286,6 +286,7 @@ int virThreadPoolSendJob(virThreadPoolPtr pool, > void *jobData) > { > virThreadPoolJobPtr job; > + struct virThreadPoolWorkerData *data = NULL; > > virMutexLock(&pool->mutex); > if (pool->quit) > @@ -298,10 +299,19 @@ int virThreadPoolSendJob(virThreadPoolPtr pool, > goto error; > } > > + if (VIR_ALLOC(data) < 0) { > + pool->nWorkers--; > + virReportOOMError(); > + goto error; > + } > + > + data->pool = pool; > + data->cond = &pool->cond; > + > if (virThreadCreate(&pool->workers[pool->nWorkers - 1], > true, > virThreadPoolWorker, > - pool) < 0) { > + data) < 0) { > pool->nWorkers--; You need to VIR_FREE(data) here... > goto error; > } > @@ -336,6 +346,7 @@ int virThreadPoolSendJob(virThreadPoolPtr pool, > return 0; > > error: > + VIR_FREE(data); ...instead of here because you can get to error label after the worker thread was successfully created (in case allocating job fails) so either data would be freed twice or the worker would read from memory that was already freed. > virMutexUnlock(&pool->mutex); > return -1; > } ACK with this fixed. Jirka