[Pixman] [PATCH] Adding TLS for Windows XP

[Tristan Schmelcher]

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

Thanks for the nice comments! I worked with Gang on this patch and
DllMain() is more or less what we're doing here. We're building Pixman
and Cairo as static libraries and linking them into a larger DLL that
provides both a 2D and 3D compositing API in a browser plugin. So
unfortunately we couldn't just put a DllMain() directly into Pixman
because our DllMain() has to handle shutdown for everything in the
DLL, not just Pixman. Instead we had to add these functions and call
out to them from our DLL's boilerplate code.

Now that I think about it though, I guess it doesn't make sense for
them to be EXPORT_SYMBOL, because there's no situation in which the
programmer should call these functions from a different module. When
Pixman is built into a DLL, the functions should only be called from
the DllMain(), and when Pixman is built directly into an executable
they should only be called from that executable's thread/process
boilerplate code. In both situations there's actually no need for the
symbols to be exported ...

What would you think about removing EXPORT_SYMBOL, prepending
underscores, and providing an example DllMain that uses the shutdown
functions for developers that are compiling Pixman as a standalone
DLL?

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

There is actually another variable: whether or not the DLL was bound
to the executable or loaded with LoadLibrary. IIRC, __declspec(thread)
works on XP in the first case but not in the second case. I'm not
familiar with the other situations though.

>
> 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, I'll check out that document.

>
>
> Thanks,
> Soren