[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: 2/25: Internal threads APIs + impl



On Tue, Jan 13, 2009 at 05:38:55PM +0000, Daniel P. Berrange wrote:
> The forthcoming set of patches will do alot of work relating to
> threads, which requires mutual exclusion primitives. This will
> include mutexes, condition variables and thread locals. We spefically
> do not need the ability to actually create thread though, at least
> not in the libvirt.so, only in the daemon.
> 
> We currently stub out all the mutex calls to no-ops when pthread.h
> is missing. This is undesirable because it means we cannot clearly
> state the thread safety rules for use of libvirt. Saying it is thread
> safe on Linux, but not on Windows is IMHO not acceptable if we want
> to have portability of apps using libvirt.
> 
> So this patch
> 
>  - Adds an internal API for mutexes, condition variables and
>    thread locals
>  - Implements this API for pthreads (covers Liunux, Solaris,
>    and all BSD variants).
>  - Implements this API for Win32 threads (covers Win32 :-)
>  - Updates all code to use these internal APIs instead of
>    pthread-XXX directly.
>  - Adds missing mutex destroy calls from earlier patches.
> 
> 
> The pthreads implementation is utterly trivial, since I purposely
> designed the internal API to map 1-to-1 onto pthreads, albeit with
> many unneccessary bits not exposed.

  Agreed with design and principle, that's basically what we ended up
doing for libxml2 but in a more limited way.

> The Win32 implementatioin is more fun. Mutexes are trivial, I
> just followed the libxml2/glib impls for that. Thread locals are
> harder since Windows doesn't provide for any destructor to be
> called when a thread exits, meaning there's a potential memory
> leak. To address this I maintain some state for Win32 thread
> locals, in particular the destructor callback. I then added a
> DllMain() impl, which triggers upon thread-exit and explicitly
> calls the destructors. This is what DBus/win32-pthread does
> for thread locals. Finally condition variables are also hard
> because there's no native condition variable impl on Windows.
> I followed the GLib condition variable impl for this which uses
> a queue of waiters, a mutex and a Win32 event for wakeup. Not
> all that hard in the end.

  Well  thread local storage is always a bit painful, whatever the
platform.

> 
> I did consider whether we could use win32-pthreads, but there's
> a few things I wasn't happy with. There are several different
> binary DLLs you have to choose between depending on what behaviour
> you want to C++ apps + exceptions. libvirt really shouldn't have
> to make that choice. More annoyingly, its pthread.h #include's
> its own config.h, so it seriously pollutes the global namespace
> with configure settings that clash with our own. This is a show
> stopper really. We also don't need the full ability to create
> threads, so wrapping Win32 thread natively wasn't very hard
> and that's what nearly every other app already does, so by using
> Win32 threads directly we should get better interoperabiltiy
> with other libraries doing the same.

  Agreed again, when the base OS has the API with proper semantic
it's really better to go native.


  went though the patch, most of it is well replacement from old
pthread_* to the new calls. The only thing which surprized me was the
Init() routines error, it's the caller which reports the error instead
of the callee, I'm wondering a bit why doing it that way.

  but looks fine, +1

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]