Re: [sdljava-users] Possible memory leak in SWIG_SDLVideo / SDL_Surface / SWIG_SDLImage
Status: Beta
Brought to you by:
ivan_ganza
Menu
▾
▴
Re: [sdljava-users] Possible memory leak in SWIG_SDLVideo / SDL_Surface / SWIG_SDLImage
|
From: Ivan Z. G. <iva...@ya...> - 2006-09-20 01:02:25
|
Hi Patric,
I didn't even have a chance to look at it properly and you've already
solved it!
Thanks so much for the detailed look you had at this. Will go over it,
it looks good. The only problem is we have to change the way SWIG
auto-generates the code. For now I'll just patch the file as you've
shown. Eventually I'll need to figure out how to get SWIG to generate
the wrapper code with that change in place. We don't really generate
the wrapper code often so it should be alright. The only time would be
when making the wrappers for a new OS or if it was required for some
other reason like a new SDL version, etc....
Will let you know when its in.
Cheers,
-Ivan/
Patric Rufflar wrote:
>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
>>
>>
>>
>>
>
>-------------------------------------------------------------------------
>Take Surveys. Earn Cash. Influence the Future of IT
>Join SourceForge.net's Techsay panel and you'll get the chance to share your
>opinions on IT & business topics through brief surveys -- and earn cash
>http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>_______________________________________________
>sdljava-users mailing list
>sdl...@li...
>https://lists.sourceforge.net/lists/listinfo/sdljava-users
>
>
|
