Re: [sdljava-users] Possible memory leak in SWIG_SDLVideo / SDL_Surface / SWIG_SDLImage

[Patric R. <pa...@ma...>]

Hi Ivan, hi all,

After playing around with the patched source I realized that
there is another critical memory problem.

At first, please have a look at the jni definition inside SWIG_SDLVideoJNI

public final static native void SDL_FreeSurface(long jarg1);
public final static native void delete_SDL_Surface(long jarg1);
public final static native long new_SDL_Surface();


The first method, SDL_FreeSurface(), is ok. It does what it name 
suggests, it calls the corresponding native SDL method.

The other two methods seems to me dummy native methods for 
contructors/destructors.  The original SDL_surface.c does not contain 
any constructors/destructors, because it's pure c.
So I think these jni calls do nothing.


Ok, let's now have a look at SDLSurface.freeSurface() which will be 
called on its finalization.

     public void freeSurface() throws SDLException {
	SWIG_SDLVideo.SDL_FreeSurface(swigSurface);
	swigSurface = null;
	pixelData   = null;
     }

As we can see, the real surface of the embedded SDL_Surface proxy object 
will be freed. But what happens if there are other valid references to 
this proxy object? It's memory pointer to the sdl surface is not valid 
anymore, freeing this object may end up in a core dump.

Now let's have a look at SDL_Surface finalization method, delete():

   public void delete() {
     if(swigCPtr != 0 && swigCMemOwn) {
       swigCMemOwn = false;
       SWIG_SDLVideoJNI.delete_SDL_Surface(swigCPtr);
     }
     swigCPtr = 0;
   }

This method calls the jni delete method, which is only a dummy. The real 
memory is not freed. This is the memory leak.

Because some SDL_Surface objects seem to be created without referenced 
by a SDLSurface. But I have to admit that I cannot give an example in 
the source right now.

To fix these two problems, I suggest the following solution:

Changes at SDLSurface:
     public void freeSurface() throws SDLException {
	SWIG_SDLVideo.SDL_FreeSurface(swigSurface); <<--delete this line
	swigSurface = null;
	pixelData   = null;
     }


Changes at SDL_Surface:
   public void delete() {
     if(swigCPtr != 0 && swigCMemOwn) {
       SWIG_SDLVideo.SDL_FreeSurface(this); // As you can see, 		 
//delete_SDL_Surface() has been replaced by the correct method.
       swigCMemOwn = false;
     }
     swigCPtr = 0;
   }


This patch will ensure, that the finalization of the proxy object 
(SDLSurface) will lead to freeing the surface not it's sdljava interface 
to it (SDL_Surface). This fixed the memory problems in my application ;)

To find those problems I am creating / deleting many surfaces in my 
application. And It seems that there is at least another (less lossy) 
memory leak which eats up memory.

And I also want to add that the methods of SWIG_SDLImage seems to be 
affected by the memory leak which I previously described, too.


Best Regards
Patric Rufflar



Ivan Z. Ganza schrieb:
> Greetings,
> 
> Thanks for finding this.  Yes this is a serious problem.
> 
> I need to do figure out how to get SWIG to set that flag to true instead 
> of false, then the leak should be eleminated.  The code there is 
> auto-generated by SWIG from the .i files.  In the worst case I can 
> manually run a script after to post-process that stuff and fix it that 
> way.  Either way it will be fixed.
> 
> Cheers,
> -Ivan/
> 
> Jesper Juul / Soup Games wrote:
> 
>> I wanted to post about this for a long time - I agre there is a leak 
>> . So far I've worked around it by wrapping all Surfaces in a custom 
>> object that calls dispose() in the finalizer.
>>
>>
>> -Jesper
>>
>>  
>>
>>> I was wondering a long time why my application needs more and more RAM.
>>> I took a while to track down a possible memory leak.
>>> It seems to be located in sdljava.x.swig.SWIG_SDLVideo .
>>>
>>> The following methods seems to be affected:
>>> SDL_ConvertSurface(), SDL_DisplayFormat() and SDL_DisplayFormatAlpha()
>>>
>>> I am not sure at the moment, but maybe other methods have the same problem:
>>> SDL_SetVideoMode(), SDL_CreateRGBSurface(), SWIG_SDL_LoadBMP(),
>>> SWIG_SDL_CreateRGBSurfaceFrom()
>>>
>>> Please have a look at the last line of all methods:
>>> return (cPtr == 0) ? null : new SDL_Surface(cPtr, false);
>>>
>>> As you can easily see, false indicates that the new surface cannot free
>>> the memory because some other surface seems to also use it. But this is
>>> not true, the surface has just been created, no other surface does own
>>> this memory area.
>>>
>>> So this bug should be fixed when setting the second argument of the
>>> surface constructors to true.
>>>
>>> Ivan, would you please verify the problem?
>>> Thank you.
>>>
>>> Best Regards,
>>> Patric
>>>    
>>>
>>
>> -------------------------------------------------------------------------
>> Using Tomcat but need to do more? Need to support web services, security?
>> Get stuff done quickly with pre-integrated technology to make your job easier
>> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
>> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
>> _______________________________________________
>> sdljava-users mailing list
>> sdl...@li...
>> https://lists.sourceforge.net/lists/listinfo/sdljava-users
>>  
>>
> 
> 
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> sdljava-users mailing list
> sdl...@li...
> https://lists.sourceforge.net/lists/listinfo/sdljava-users
> 
>