[Pixman] [BUG REPORT] pixman_glyph_cache_create leaks memory that it allocates

[Siarhei Siamashka]

On Sun, 17 Sep 2017 13:18:01 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> Hi Yuri,
> 
> On 15 September 2017 at 22:41, Yuri <yuri at rawbw.com> wrote:
> > google-perftools memory profile on a gtkmm-based app shows that there is
> > 0.3MB of memory left by pixman_glyph_cache_create.
> >
> >
> > pixman should free all memory that it allocates. For example, if the app
> > will load the gtkmm UI dynamically, this memory will be permanently lost
> > every time, resulting in the memory leak.
> >
> >
> > pixman-0.34.0
> >  
> 
> Keep in mind that I'm not a pixman developer, so take the following
> with a grain of salt.
> 
> Skimming through the pixman code - this seems like an user error.
> Namely are you sure the application has not forgot to free the cache
> before destroying it?
> 
> Pay special attention to stderr, as pixman_glyph_cache_destroy() is
> invoked. You should see a lovely "*** BUG ***" with extra information.
> 
> As a sanity check - the following shows no leaks in the mentioned code path*.
> 
> $ cat pp.c
> // gcc -O0 -I/usr/include/pixman-1 -lpixman-1 -o pp-test && valgrind pp-test
> #include <pixman.h>
> 
> int
> main(void)
> {
>        pixman_glyph_cache_destroy(pixman_glyph_cache_create());
>        return 0;
> }
> 
> HTH
> Emil

Yes, most of the problems with pixman are user errors nowadays.

> * There are some 'leaks' due to the custom DSO constructor, but let's
> look at that separately.

Indeed, it's a rather tricky and fragile stuff, which is also system and
compiler dependent. We need to do some init/deinit stuff at the time of
library load/unload (the pixman implementation pointer) and also at the
time of thread creation/destruction too (the per-thread fast path
cache).

If we only had to support just Linux as the operating system and only
GCC as the compiler, then everything would be rather straightforward.
But pixman works in other environments too.

Can Yuri tell us more about his dynamic load/reload use case? So far
I'm not aware of anyone doing this in practice.

Maybe the right solution is to add explicit pixman_init() and
pixman_destroy() functions. But we need to support existing
applications too and we can't expect them to start using these
new functions without enforcing some changes. That's a kind of
historical baggage, and nobody feels like opening this can of
worms.

-- 
Best regards,
Siarhei Siamashka