[libvirt] [PATCH 07/11] Introduce portability APIs for creating threads

Matthias Bolte matthias.bolte at googlemail.com
Fri Nov 5 23:41:11 UTC 2010


2010/11/2 Daniel P. Berrange <berrange at redhat.com>:
> The util/threads.c/h code already has APIs for mutexes,
> condition variables and thread locals. This commit adds
> in code for actually creating threads.
>

> +
> +void virThreadSelf(virThreadPtr thread)
> +{
> +    thread->thread = pthread_self();
> +}
> +
> +bool virThreadIsSelf(virThreadPtr thread)
> +{
> +    return pthread_self() == thread->thread ? true : false;
> +}

The pthread_self manpage says that it's not safe to compare
pthread_t's using == and advises to use pthread_equal for this.

> +
> +void virThreadJoin(virThreadPtr thread)
> +{
> +    pthread_join(thread->thread, NULL);
> +}
>
>  int virThreadLocalInit(virThreadLocalPtr l,
>                        virThreadLocalCleanup c)


> diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
> index fe1fcd0..3f69f41 100644
> --- a/src/util/threads-win32.c
> +++ b/src/util/threads-win32.c
> @@ -21,6 +21,8 @@
>
>  #include <config.h>
>
> +#include <process.h>
> +
>  #include "memory.h"
>
>  struct virThreadLocalData {
> @@ -205,6 +207,51 @@ void virCondBroadcast(virCondPtr c)
>  }
>
>
> +struct virThreadArgs {
> +    virThreadFunc func;
> +    void *opaque;
> +};
> +
> +static unsigned int __stdcall virThreadHelper(void *data)
> +{
> +    struct virThreadArgs *args = data;
> +    args->func(args->opaque);
> +    return 0;
> +}
> +
> +int virThreadCreate(virThreadPtr thread,
> +                    bool joinable ATTRIBUTE_UNUSED,
> +                    virThreadFunc func,
> +                    void *opaque)
> +{
> +    struct virThreadArgs args = { func, opaque };
> +    thread->thread = (HANDLE)_beginthreadex(NULL, 0, virThreadHelper, &args, 0, NULL);
> +    return 0;
> +}

When you create a non-joinable thread then you won't call
virThreadJoin on it and you''ll leak the handle. Therefore, we should
use _beginthread here instead of _beginthreadex when creating a
non-joinable thread, because _endthread will close the thread's handle
when the thread's function returns.

See http://msdn.microsoft.com/en-us/library/kdzttdcb(VS.71).aspx

> +
> +void virThreadSelf(virThreadPtr thread)
> +{
> +    HANDLE handle = GetCurrentThread();
> +    HANDLE process = GetCurrentProcess();
> +
> +    DuplicateHandle(process, handle, process,
> +                    &thread->thread, 0, FALSE,
> +                    DUPLICATE_SAME_ACCESS);
> +}

If virThreadJoin is not called on virThreadPtr initialized by
virThreadSelf then the duplicated handle leaks.

See http://msdn.microsoft.com/en-us/library/ms683182(VS.85).aspx

> +bool virThreadIsSelf(virThreadPtr thread)
> +{
> +    virThread self;
> +    virThreadSelf(&self);
> +    return self.thread == thread->thread ? true : false;
> +}

Therefore, this definitely leaks a handle per call.

> +void virThreadJoin(virThreadPtr thread)
> +{
> +    WaitForSingleObject(thread->thread, INFINITE);
> +    CloseHandle(thread->thread);
> +}
> +

This might create really subtle double-CloseHandle bugs when you call
it twice on the same virThreadPtr or call it accidentally on an
non-joinable thread that already exited and _endthread had close the
handle. In both cases Windows might already reused the closed handle
and the second call to CloseHandle might close something completely
unrelated.

Matthias




More information about the libvir-list mailing list