[Pixman] [PATCH] Adding TLS for Windows XP

[Gang Ji]

Hi Soeren,

Thank you very much for your comments!

We don't want to add another API either.  It would be nice to make the
library multi-thread safe in Windows XP and no memory leaks at the same
time.  One question regarding your suggestions of freeing the memory in DLL
unload.  TlsFree will still need to know all the TLS indices (
http://msdn.microsoft.com/en-us/library/ms686804(v=vs.85).aspx) and Pixman
core will have that information.  So we might still need a new API.

Thanks a lot for your help!

Best regards,
Gang

On Wed, Mar 30, 2011 at 6:19 AM, Soeren Sandmann <sandmann at cs.au.dk> wrote:

> Hi,
>
> > It turned out that Windows XP does't support __declspec(thread).  In
> order to
> > make a multi-thread safe app using pixman, we tried to add TLS for
> Windows XP.
> > We have added using TLS primitives and tried to avoid memory leak during
> > cleaning up.
>
> I'm of two minds regarding this patch. On the one hand, it's definitely
> good to get these TLS issues under control on Windows, so thanks for
> looking at that.
>
> On the other hand, I really don't want to export new API that an
> application has to call simply for correct operation.
>
> If you use pixman through cairo, for example, it is currently completely
> safe to ignore pixman's existence. This is an important guarantee to
> give because it allows us to update the pixman ABI at a later point
> without affecting cairo applications.
>
> Or suppose some other library has spawned a thread that uses pixman for
> compositing. In general, the application has no way to know when that
> thread exited so it can't know when to call these new functions.
>
> Since you mention that __declspec(thread) doesn't work on XP, I assume
> you are using pixman in a dynamically linked DLL. If so, a way to solve
> your issue without adding new API might be to add a DllMain() function
> and then clean up the thread local storage when it is called with
> THREAD_DETACH.
>
> Another possibility would be to avoid TLS altogether and instead just
> use global variables protected by locks. Lock striping could be used to
> prevent (CPU-)cache thrashing.
>
> Regarding which variation of TLS works on which versions of Windows,
> there are several separate variables:
>
> - Whether the binary is statically or dynamically linked
>
> - Which compiler is used (MSVC vs. MinGW32 4.5 vs. MinGW32 4.4)
>
> - Which operating system is used (XP/2003 vs. later)
>
> - Which variation of TLS (__thread, __declspec(thread), DllMain() +
> TlsAlloc*)
>
> - Whether the operating system is 32 or 64 bit
>
> It would be really useful to have a table of these combinations showing
> what works and what doesn't. That will make it much simpler to decide
> which combinations can actually be reasonably supported without
> requiring new public API.
>
> See also
>
>    https://bugs.freedesktop.org/show_bug.cgi?id=34842
>
> Regardless of all that, though, this patch clearly needs cleaning up
> according to the CODING_STYLE document.
>
>
> Thanks,
> Soren
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20110405/95891220/attachment.htm>