Re: memory leak - cairo/x11 backend

[Eric Wasylishen]

Hi Riccardo, +Fred +gnustep-dev

On Dec 18, 2013, at 7:40 AM, Riccardo Mottola <address@hidden> wrote:

> Hi,
> 
> Eric Wasylishen wrote:
>> 
>> Hi Riccardo,
>> 
>> As a starting point for debugging I would put NSLog's in 
>> XGCairoModernSurface -initWithDevice: and -dealloc. This is where we retain 
>> / release the Cairo surface that is holding memory in the x server.
>> 
>> Also I would put logging in -NSImage set name:, log [nameDict allKeys] to 
>> see which images are kept in memory by nameDict.
>> 
> I did so, in setName.. It does not get called from laternaMagica when 
> switching images. WHile the application loads and shows its windows I see 
> stuff like:
> 
> 2013-12-18 15:36:39.356 LaternaMagica[18906] NSimage setName, keys: 
> ("common_ret H", "common_Nibble", "common_HomeDirectory", NSApplicationIcon, 
> "common_UnknownT ool", GSMenuArrow, "common_Mount", "common_DownloadFolder", 
> "common_ArrowDown", "laternamagica_48.tif", "common_Home", 
> "common_ArrowLeftH", "common_Close", "com mon_3DArrowLeft", "common_Desktop", 
> "common_ImageFolder", GSMenuSelected, "commo n_3DArrowRightH", GSSwitch, 
> GSSwitchSelected, "common_GSFolder", "common_Dimple" , "common_Unmount", 
> "common_DimpleHoriz", "common_ArrowRightH", "common_DocsFold er", 
> "common_Unknown", "common_ArrowUpH", GSMenuMixed, NSFolder, "common_ArrowUp 
> ", "common_3DArrowUp", "common_ArrowRight", "common_3DArrowDown", 
> "common_ArrowD ownH", "common_ret", "common_ArrowLeft", "common_SliderHoriz")
> 
> 
> however when I load images from disk and cycle between them, nothing gets 
> printed, meaning that setName doesn't get called.
> 
> Riccardo

Actually, I was on the wrong track; I just found the problem.

During the creation of a window, we run -[XGServerWindow 
setWindowdevice:forContext:]. The problem is, inside that method we call [self 
_createBuffer] which does window->buffer = XCreatePixmap(…), but the [self 
_createBuffer] call is *before* window->gdriverProtocol is initialized. 
window->gdriverProtocol is set from the -init… method of XGCairoModernSurface.

The call sequence that causes the leak looks like:

-[XGServerWindow setWindowdevice:forContext:]
-[XGServerWindow _createBuffer]
(performs window->buffer = XCreatePixmap because window->gdriverProtocol == 0)
GSSetDevice()
-[XGCairoModernSurface initWIthDevice:]
(sets window->gdriverProtocol to GDriverHandlesExpose | GDriverHandlesBacking)
…later….
-[XGServerWindow termwindow] doesn’t free window->buffer because 
window->gdriverProtocol has the GDriverHandlesBacking bit set.



As a quick fix, I reordered this section of  -[XGServerWindow 
setWindowdevice:forContext:]

  if (window->buffer == 0)
    {
      [self _createBuffer: window];
    }

  [self styleoffsets: &l : &r : &t : &b
                    : window->win_attrs.window_style : window->ident];
  GSSetDevice(ctxt, window, l, NSHeight(window->xframe) + b);

to:

  [self styleoffsets: &l : &r : &t : &b
                    : window->win_attrs.window_style : window->ident];
  GSSetDevice(ctxt, window, l, NSHeight(window->xframe) + b);
  if (window->buffer == 0)
    {
      [self _createBuffer: window];
    }


which fixed the leak, however it causes apps to segfault with the xlib backend. 
Probably some code in -styleoffsets or GSSetDevice attempts to use 
window->buffer. 

I just committed a more foolproof fix, which is just disabling all of the code 
in -_createBuffer if we are building with the cairo backend. As I noted in the 
comment there, I don’t like the window->gdriverProtocol abstraction as it’s 
trying to handle different library configurations in “too dynamic” of a way. 

Testing is welcome; the leak is gone for me and LaternaMagica seems a lot 
faster too.

Cheers,
Eric

PS, ignore my comments about caching, for some reason I was thinking that all 
NSImages would go in namedict. What we have now is more or less fine, UI 
resources loaded with +imageNamed: are kept cached in nameDict and others are 
not.