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

2010/11/2 Daniel P. Berrange <berrange(a)redhat.com&gt;: > +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

> +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.