[Pixman] Valgrind-clean pixman

Siarhei Siamashka siarhei.siamashka at gmail.com
Fri Aug 27 06:58:04 PDT 2010


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.

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.

-- 
Best regards,
Siarhei Siamashka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20100827/c8653ef8/attachment.pgp>


More information about the Pixman mailing list