[libvirt] [PATCH] Threadpool: Initialize new dynamic workers
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--; goto error; } @@ -336,6 +346,7 @@ int virThreadPoolSendJob(virThreadPoolPtr pool, return 0; error: + VIR_FREE(data); virMutexUnlock(&pool->mutex); return -1; } -- 1.7.3.4
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
On 07.09.2011 14:22, Jiri Denemark wrote:
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
Thanks, pushed with that fixed. Michal
participants (2)
-
Jiri Denemark -
Michal Privoznik