[Pixman] Valgrind-clean pixman

[Andrea Canciani]

On Fri, Aug 27, 2010 at 3:58 PM, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
> On Thursday 26 August 2010 20:40:08 Andrea Canciani wrote:
>> Currently pixman allocates dynamically its implementations, but does
>> not free them.
>> This is not a true memory leak, since implementations are only
>> allocated once, but
>> many debugging tools will report the asyimmetric malloc/free as a memory
>> leak. I wrote a patch that avoids allocating pixman_implementation_t's (by
>> declaring them
>> to be static) in
>> http://cgit.freedesktop.org/~ranma42/pixman/commit/?h=wip/valgrind2
>> More general solutions exists (teardown/reset functions), but
>> currently they would not
>> provide additional advantages and would be more complicated.
>> Objections, suggestions, comments, reviews,... are appreciated.
>
> This code which is setting a global implementation pointer is also not quite
> thread safe (though very unlikely to cause any practical problems other than a
> bit bigger one time memory leak). I did not look at it in details, but with
> your changes for the use of static structures, it may become a bit worse
> because two or more concurrent threads now can potentially try to initialize
> the same data structures at once when setting up implementation pointer.
Pixman is not thread safe right now (reference counting, in particular). When we
fix (probably by using atomics) reference count, it should be very easy to make
implementations thread safe, too.
I'm working on a library of simple inline functions (based on cairo
atomic and mutex
implementations) to share some common utility functions which have different
availability on different platforms (right now: atomics, wideint, mutexes).
I hope to make it available soon (after I get autotools do the right
things to make it
work).
>
> While we are at it, helgrind complains a lot about 'blitters-test' and
> 'scaling-test' programs. These messages are related to implementation setup and
> can be suppressed if a dummy compositing operation is done before starting the
> main loop which uses OpenMP there.
>
> Having something like pixman_init()/pixman_cleanup() functions could solve all
> the problems, but it's an API change. Other solutions (using atexit(), using
> mutexes or atomic oparations when initializing implementation, putting
> 'global_implementation' into thread local storage, etc.) seem to all have some
> risk of introducing portability issues and/or add extra runtime overhead.
Would using library ctor/dtor be ok? They should be available both on win32
(DllMain) and as a gcc attribute and would allow to initialize things in a very
simple way (without having to use any synchronization).

See:
http://msdn.microsoft.com/en-us/library/ms682583(VS.85).aspx
http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html

We probably want to implement DllMain on win32 to allocate/free TLS anyway.