threads: check for failure to set thread-local value

We had a memory leak on a very arcane OOM situation (unlikely to ever
hit in practice, but who knows if libvirt.so would ever be linked
into some other program that exhausts all thread-local storage keys?).
I found it by code inspection, while analyzing a valgrind report
generated by Alex Jia.

* src/util/threads.h (virThreadLocalSet): Alter signature.
* src/util/threads-pthread.c (virThreadHelper): Reduce allocation
lifetime.
(virThreadLocalSet): Detect failure.
* src/util/threads-win32.c (virThreadLocalSet): Likewise.
(virCondWait): Fix caller.
* src/util/virterror.c (virLastErrorObject): Likewise.
This commit is contained in:
Eric Blake 2011-12-20 15:06:47 -07:00
parent 91f79d27cc
commit 927cfaf467
4 changed files with 21 additions and 9 deletions

View File

@ -154,8 +154,11 @@ struct virThreadArgs {
static void *virThreadHelper(void *data) static void *virThreadHelper(void *data)
{ {
struct virThreadArgs *args = data; struct virThreadArgs *args = data;
args->func(args->opaque); struct virThreadArgs local = *args;
/* Free args early, rather than tying it up during the entire thread. */
VIR_FREE(args); VIR_FREE(args);
local.func(local.opaque);
return NULL; return NULL;
} }
@ -249,7 +252,12 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return pthread_getspecific(l->key); return pthread_getspecific(l->key);
} }
void virThreadLocalSet(virThreadLocalPtr l, void *val) int virThreadLocalSet(virThreadLocalPtr l, void *val)
{ {
pthread_setspecific(l->key, val); int err = pthread_setspecific(l->key, val);
if (err) {
errno = err;
return -1;
}
return 0;
} }

View File

@ -155,7 +155,10 @@ int virCondWait(virCondPtr c, virMutexPtr m)
if (!event) { if (!event) {
return -1; return -1;
} }
virThreadLocalSet(&virCondEvent, event); if (virThreadLocalSet(&virCondEvent, event) < 0) {
CloseHandle(event);
return -1;
}
} }
virMutexLock(&c->lock); virMutexLock(&c->lock);
@ -376,7 +379,7 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return TlsGetValue(l->key); return TlsGetValue(l->key);
} }
void virThreadLocalSet(virThreadLocalPtr l, void *val) int virThreadLocalSet(virThreadLocalPtr l, void *val)
{ {
TlsSetValue(l->key, val); return TlsSetValue(l->key, val) == 0 ? -1 : 0;
} }

View File

@ -104,7 +104,7 @@ typedef void (*virThreadLocalCleanup)(void *);
int virThreadLocalInit(virThreadLocalPtr l, int virThreadLocalInit(virThreadLocalPtr l,
virThreadLocalCleanup c) ATTRIBUTE_RETURN_CHECK; virThreadLocalCleanup c) ATTRIBUTE_RETURN_CHECK;
void *virThreadLocalGet(virThreadLocalPtr l); void *virThreadLocalGet(virThreadLocalPtr l);
void virThreadLocalSet(virThreadLocalPtr l, void*); int virThreadLocalSet(virThreadLocalPtr l, void*) ATTRIBUTE_RETURN_CHECK;
# ifdef WIN32 # ifdef WIN32
# include "threads-win32.h" # include "threads-win32.h"

View File

@ -267,7 +267,8 @@ virLastErrorObject(void)
if (!err) { if (!err) {
if (VIR_ALLOC(err) < 0) if (VIR_ALLOC(err) < 0)
return NULL; return NULL;
virThreadLocalSet(&virLastErr, err); if (virThreadLocalSet(&virLastErr, err) < 0)
VIR_FREE(err);
} }
return err; return err;
} }
@ -612,7 +613,7 @@ virDispatchError(virConnectPtr conn)
virErrorFunc handler = virErrorHandler; virErrorFunc handler = virErrorHandler;
void *userData = virUserData; void *userData = virUserData;
/* Should never happen, but doesn't hurt to check */ /* Can only happen on OOM. */
if (!err) if (!err)
return; return;