[libvirt] [PATCH] daemon: plug a memory leak
* daemon/libvirtd.c (qemudStartWorker, qemudStartEventLoop): Avoid leaking pthread_attr resources. --- daemon/libvirtd.c | 36 ++++++++++++++++++++++++++---------- 1 files changed, 26 insertions(+), 10 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 2914f2f..14777dd 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1600,15 +1600,20 @@ static void *qemudWorker(void *data) } } -static int qemudStartWorker(struct qemud_server *server, - struct qemud_worker *worker) { +static int +qemudStartWorker(struct qemud_server *server, + struct qemud_worker *worker) +{ pthread_attr_t attr; - pthread_attr_init(&attr); + int ret = -1; + + if (pthread_attr_init(&attr) != 0) + return -1; /* We want to join workers, so don't detach them */ /*pthread_attr_setdetachstate(&attr, 1);*/ if (worker->hasThread) - return -1; + goto cleanup; worker->server = server; worker->hasThread = 1; @@ -1621,10 +1626,13 @@ static int qemudStartWorker(struct qemud_server *server, worker) != 0) { worker->hasThread = 0; worker->server = NULL; - return -1; + goto cleanup; } - return 0; + ret = 0; +cleanup: + pthread_attr_destroy(&attr); + return ret; } @@ -2414,9 +2422,14 @@ cleanup: } -static int qemudStartEventLoop(struct qemud_server *server) { +static int +qemudStartEventLoop(struct qemud_server *server) +{ pthread_attr_t attr; - pthread_attr_init(&attr); + int ret = -1; + + if (pthread_attr_init(&attr) != 0) + return -1; /* We want to join the eventloop, so don't detach it */ /*pthread_attr_setdetachstate(&attr, 1);*/ @@ -2424,11 +2437,14 @@ static int qemudStartEventLoop(struct qemud_server *server) { &attr, qemudRunLoop, server) != 0) - return -1; + goto cleanup; server->hasEventThread = 1; - return 0; + ret = 0; +cleanup: + pthread_attr_destroy(&attr); + return ret; } -- 1.7.3.3
2010/12/13 Eric Blake <eblake@redhat.com>:
* daemon/libvirtd.c (qemudStartWorker, qemudStartEventLoop): Avoid leaking pthread_attr resources. --- daemon/libvirtd.c | 36 ++++++++++++++++++++++++++---------- 1 files changed, 26 insertions(+), 10 deletions(-)
ACK. Matthias
On 12/13/2010 04:20 PM, Matthias Bolte wrote:
2010/12/13 Eric Blake <eblake@redhat.com>:
* daemon/libvirtd.c (qemudStartWorker, qemudStartEventLoop): Avoid leaking pthread_attr resources. --- daemon/libvirtd.c | 36 ++++++++++++++++++++++++++---------- 1 files changed, 26 insertions(+), 10 deletions(-)
ACK.
Thanks. Hmm, src/util/threads-pthread.c had the same leak; I'm squashing this in, then pushing. diff --git i/src/util/threads-pthread.c w/src/util/threads-pthread.c index c406f27..1653396 100644 --- i/src/util/threads-pthread.c +++ w/src/util/threads-pthread.c @@ -157,9 +157,15 @@ int virThreadCreate(virThreadPtr thread, { struct virThreadArgs *args; pthread_attr_t attr; - pthread_attr_init(&attr); - if (VIR_ALLOC(args) < 0) + int ret = -1; + int err; + + if ((err = pthread_attr_init(&attr)) != 0) goto cleanup; + if (VIR_ALLOC(args) < 0) { + err = ENOMEM; + goto cleanup; + } args->func = func; args->opaque = opaque; @@ -167,14 +173,19 @@ int virThreadCreate(virThreadPtr thread, if (!joinable) pthread_attr_setdetachstate(&attr, 1); - int ret = pthread_create(&thread->thread, &attr, virThreadHelper, args); - if (ret != 0) { + err = pthread_create(&thread->thread, &attr, virThreadHelper, args); + if (err != 0) { VIR_FREE(args); - errno = ret; - return -1; + goto cleanup; } /* New thread owns 'args' in success case, so don't free */ - return 0; + + ret = 0; +cleanup: + pthread_attr_destroy(&attr); + if (ret < 0) + errno = err; + return ret; } void virThreadSelf(virThreadPtr thread) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Dec 13, 2010 at 03:49:15PM -0700, Eric Blake wrote:
* daemon/libvirtd.c (qemudStartWorker, qemudStartEventLoop): Avoid leaking pthread_attr resources. --- daemon/libvirtd.c | 36 ++++++++++++++++++++++++++---------- 1 files changed, 26 insertions(+), 10 deletions(-)
NB this code that you are changing has all been deleted by my RPC patches, and the new job use the threadpool APIs instead of pthread. Daniel
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte