nanogui: Thread: XCreateWindow/XDestroyWindow memory leak?

I have a FLTK program that works with microwin + nxlib and every time I
create and destroy a window, I lose few hundred bytes of heap memory.
This memory loss becomes significant and unacceptable after many window
operations.

Initially, I blamed it on FLTK but after debugging it for a while, I
conclude (?) that the problem is with nxlib.

I can see a FIXME message in DestWind.c to destroy various structures
associated with a window.
This would explain my problem if I knew what structures are involved.

Any hints so I can fix this problem?
Martin

> I can see a FIXME message in DestWind.c to destroy various structures
associated with a window.

Hmmm.  The only auxillary structures around a Window type in NXLIB
have to do with window properties, see ChProperty.c.  However,
these are properly deallocated, and only used if XChangeProperty
is used (please check).

There are two known memory leaks in NXLIB, having to do with
clip regions: XSetClipMask and XSetClipRegion, both in SetClip.c.

Add a printf() to the above functions to see whether these are called
from FLTK.  Should this be the case, then I'll have to research
why the regions couldn't be deleted within the NXLIB functions.
I think it has to do with the microwindows GrXXX clip functions
requiring the region to exist after passing to GrSetGCRegion().

The fix here may finally be (without modifying microwindows) to
keep any region allocated for a GC in the local NXLIB GC struct,
and then release it when the GC is deallocated.

Regards,

Greg

> Hmmm.  The only auxillary structures around a Window type in NXLIB
have to do with window properties, see ChProperty.c.  
> However, these are properly deallocated, and only used if
XChangeProperty is used (please check).

Well, there is a bug in ChProperty.c and _nxDelAllProperty() never
deletes anything.
The problem is that _nxDelAllProperty() checks if (win->w == w) but
win->w is never set.
To fix this you have to add:
      win->w = w;
in _nxAddProperty(), line 47, right before:
      if (!win->properties)

After this fix, the _nxDelAllProperty() deletes properties like it is
suppose to.
This reduced the amount of memory not deallocated by windows but there
is still something not being deallocated.

I have a question:
I can see that FLTK sends some properties with property name = NULL.
Is this legal and will it cause any problems in nxlib?

My thinking is that if the NULL properties represent different
properties, then nxlib will not recognize them as being different.
But then I do not know much about it.
Martin

> The problem is that _nxDelAllProperty() checks if (win->w == w) but
win->w is never set.

Good catch!  I'll add this to CVS.


> This reduced the amount of memory not deallocated by windows but there
is still something not being deallocated.

Put a printf() in the other XSetClip function to see whether
this might be the other leak, or whether we should still look
hard at ChProperties.c


> I can see that FLTK sends some properties with property name = NULL.
Is this legal and will it cause any problems in nxlib?
My thinking is that if the NULL properties represent different
properties, then nxlib will not recognize them as being different.

The basic idea behind properties is to allow a list of
associations of names and values attached and accessible
from a Window.  The property name is converted
to an Atom by using XInternAtom which takes a string
name and returns an integer Atom.  This Atom is then used
as the name to XChangeProperty, with byte data passed
for the value.

Thus, if the Atom name itself is 0 (NULL), passed to 
XChangeProperty, then this likely should be checked
for first, and do nothing.  If that's not what you're
saying with property name=NULL above, then
please explain.

So, we may also be associating some NULL properties
with data, but these should have been deleted when
the _nxDelProperty is called.

Another fix is that we should check for data=NULL
and handle that accordingly in the _nxAddProperty
code.

Regards,

Greg

on 13.11.07 21.34 Greg Haerr wrote:
>> This reduced the amount of memory not deallocated by windows but there
> is still something not being deallocated.
> 
> Put a printf() in the other XSetClip function to see whether
> this might be the other leak, or whether we should still look
> hard at ChProperties.c
> 

Some long time ago I had that problem with XSetClip() and FLTK too and made a modification that 
solved the problem for me.

Hermann


int XSetClipMask(Display *display, GC gc, Pixmap mask)
{
         static GR_GC_ID     last_gc_gid = 0;
	static GR_REGION_ID last_region = 0;
	
	GR_REGION_ID	r;
	GR_WINDOW_INFO	info;

         // to avoid a memory leak, delete region if last region hasn't been deleted before
         if ( (last_gc_gid == gc->gid) && (last_region != 0) )
	{
           GrDestroyRegion(last_region);
//printf("XSetClipMask remove %d %d %d\n", mask, last_gc_gid, last_region);
	  last_region = 0;
	  last_gc_gid = 0;
	}
	else
	{
//printf("XSetClipMask NO remove %d %d %d\n", mask, last_gc_gid, last_region);
	}
	
	if (mask == None) {
		GrSetGCRegion(gc->gid, 0);
		return 1;
	}

	GrGetWindowInfo(mask, &info);
	r = GrNewRegionFromPixmap(mask, 0, 0, info.width, info.height);
	GrSetGCRegion(gc->gid, r);

	last_region = r;
	last_gc_gid = gc->gid;
//printf("XSetClipMask crate  %d %d %d\n", mask, last_gc_gid, last_region);

	/*GrDestroyRegion(r);*/	/* FIXME: LEAK: can't destroy region here...*/
	return 1;
}

> Some long time ago I had that problem with XSetClip() and FLTK too and
made a modification that solved the problem for me.

I already have a similar fix in my code, but I tried yours just in case
and the results are the same.
The leak is about 200-300 bytes per window.
I am still looking for it. (XChangeProperty?)
Martin

> I already have a similar fix in my code, but I tried yours just in case
and the results are the same.
The leak is about 200-300 bytes per window.
I am still looking for it. (XChangeProperty?)

Perhaps comment out XChangeProperty from doing anything,
see if that results in fixing it.  Then we'll know whether
we need to look more closely at that portion.

Regards

Greg

> Perhaps comment out XChangeProperty from doing anything, see if that
results in fixing it.  Then we'll know whether we need to look more
closely at that portion.


I did not try it yet but I believe I found another leak in
XChangeProperty.
When XChangeProperty is called a few times with property set to some
(different) number and XGetAtomName() returns NULL, then
_nxDelAllProperty only deletes the first one.
Looks like nxlib combines all AtomNames = NULL into one even though they
have different Atom properties.
How can we fix this?

This observation is based on logging all calls to XChangeProperty and
_nxDelAllProperty. _nxDelProperty was not called at all.
Martin

> Perhaps comment out XChangeProperty from doing anything, see if that
results in fixing it.  Then we'll know whether we need to look more
closely at that portion.

Disabling XChangeProperty stopped the leak.
It must be the NULL XGetAtomNames.
How to fix it properly? Hints?

> When XChangeProperty is called a few times with property set to some
(different) number and XGetAtomName() returns NULL, then
_nxDelAllProperty only deletes the first one.

I don't quite understand what XGetAtomName has to do with
this, is this a seperate call FLTK makes?  XChangeProperty
uses Atoms, not names.  Please explain


> Looks like nxlib combines all AtomNames = NULL into one even though they
have different Atom properties.

I think there's two bugs here.  First, if XChangeProperty is 
called with an Atom of 0, then it should just return.  This
should be tested before doing anything.

Secondly, if XChangeProperty is called with an Atom with
the same value, the property value should be overwritten (or
prepend/appended).  In no case should the property be
created twice.

Does this make sense given your observations?

Regards,

Greg