[Pixman] Windows XP delay-loaded dll patch

[Andrea Canciani]

On Thu, Dec 12, 2013 at 1:11 AM, Alex Christensen <
alex.christensen at flexsim.com> wrote:

> My coworker Anthony and I have been working on the WinCairo port of WebKit
> which includes pixman.  We were having it crash when delay loading the dlls
> on Windows XP because of improper usage of __declspec(thread).  We are
> using the MinGW code with Visual Studio.  I'm not familiar with pixman
> coding style, but here's something similar to our fix.  The windows.h stuff
> is a little hacky, but the important part is the removal of the
> __declspec(thread).
>

It looks like you are hitting the issue I mentioned here:
http://lists.freedesktop.org/archives/pixman/2013-October/003016.html

The patch fixes the issue by using the dynamic TLS allocation even on MSVC,
but unfortunately the patch does not fix the long-standing issue of TLS
leakage on win32: each thread that uses pixman would allocate a fast-path
table, but it would never deallocate it.
https://bugs.freedesktop.org/show_bug.cgi?id=34842

I have a (somewhat outdated) branch which also provides TLS deallocation:
http://cgit.freedesktop.org/~ranma42/pixman/log/?h=wip/simpleops-to-master
Shall I rebase it send in a patchset?
It depends on the simpleops header library, whose latest changes are in the
c11 branch (which, as the name states, tries to have a C11-like API):
http://cgit.freedesktop.org/~ranma42/simpleops/log/?h=c11

I agree that the simpleops code is quite unconvenient to view manually,
because it is mostly based on templates, but the library includes a view
"utility" that can be used to compare different implementations (it aligns
them and highlights the differences).
For example, the TSS implementations can be seen here:
http://people.freedesktop.org/~ranma42/simpleops/index.html?tss/impl/_dispatch.hor,
if you checked out simpleops locally, opening
file:///path/to/simpleops/index.html?atomic/impl/_dispatch.h
Added bonuses coming with simpleops are: the API is documented (ok,
somewhat... threading is quite complicated and the C11 specification
provides a much more extensive explanation of memory model etc) and there
is a test suite (
http://cgit.freedesktop.org/~ranma42/simpleops-test/log/?h=c11 ) which
tries to set up some not-so-obvious configurations (combinations of static
& dynamic linking, doing things at library load time, etc).



>
> http://msdn.microsoft.com/en-us/library/yx1x886y.aspx
> https://github.com/bfulgham/WinCairoRequirements
>
> There are probably not many people using pixman on Windows XP with delay
> loaded dlls, but I think a fix similar to this ought to be upstreamed:
>
> diff --git a/pixman-compiler.h b/pixman-compiler.h
> --- a/pixman-compiler.h
> +++ b/pixman-compiler.h
> @@ -118,6 +118,6 @@
>  #   define PIXMAN_GET_THREAD_LOCAL(name) \
>      (&name)
>
> -#elif defined(__MINGW32__)
> +#elif defined(__MINGW32__) || defined(_MSC_VER)
>
>  #   define _NO_W32_PSEUDO_MODIFIERS
> @@ -122,3 +122,5 @@
>
>  #   define _NO_W32_PSEUDO_MODIFIERS
> +
> +#if !defined(_MSC_VER)
>  #   include <windows.h>
> @@ -124,4 +126,5 @@
>  #   include <windows.h>
> +#endif
>
>  #   define PIXMAN_DEFINE_THREAD_LOCAL(type, name) \
>      static volatile int tls_ ## name ## _initialized = 0; \
> @@ -171,13 +174,6 @@
>  #   define PIXMAN_GET_THREAD_LOCAL(name) \
>      tls_ ## name ## _get ()
>
> -#elif defined(_MSC_VER)
> -
> -#   define PIXMAN_DEFINE_THREAD_LOCAL(type, name) \
> -    static __declspec(thread) type name
> -#   define PIXMAN_GET_THREAD_LOCAL(name) \
> -    (&name)
> -
>  #elif defined(HAVE_PTHREADS)
>
>  #include <pthread.h>
> diff --git a/pixman-implementation.c b/pixman-implementation.c
> --- a/pixman-implementation.c
> +++ b/pixman-implementation.c
> @@ -63,6 +63,10 @@
>      } cache [N_CACHED_FAST_PATHS];
>  } cache_t;
>
> +#if defined(_MSC_VER)
> +#   include <windows.h>
> +#endif
> +
>  PIXMAN_DEFINE_THREAD_LOCAL (cache_t, fast_path_cache);
>
>  static void
>
>
> --
>
>
>
> Alex Christensen
>
> FlexSim Software Products, Inc.
>
> *1577 North Technology Way | Building A | Suite 2300 | Orem, Utah 84097*
>
> *Voice: 801-224-6914 | Fax: 801-224-6984*
>
> *Email:* alexc at flexsim.com <kimw at flexsim.com>
>
> *URL:* www.flexsim.com
>
>
>
>
> ----------------------------------------------------------------------------------------
>
> This message may contain confidential information, and is intended
>
> only for the use of the individual(s) to whom it is addressed.
>
> ----------------------------------------------------------------------------------------
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20131212/c6714c36/attachment.html>