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-24 17:26:36
|
Greetings,
I've implemented the fixes as suggested and the code is checked into CVS.
Before I checked in the changes I tagged the source as
"PRE_MEMORY_LEAK_FIX" and afterwards I tagged as "POST_MEMORY_LEAK_FIXES".
Please let me know if there are any problems.
Cheers,
-Ivan/
Patric Rufflar wrote:
>Hi Ivan,
>
>You're welcome.
>I'm glad that I could help improving sdljava. :)
>
>By the way: SWIG_SDLTTF and SWIG_SDLGfx contain faulty constructors of
>SDL_Surfaces, too. Please set false to true to fix this.
>
>Now I cannot see any other memory leaks (for now ;-) )
>
>And for now, I can't await that other points of my wishlist will come true :)
>- new sdljava release :)
>- new sdljava release for windows (oh, please. But maybe I have some free time
>in the future and have a look how to set up a suitable build environment)
>- Support ffmpeg
>- full YUV Support (or do we have a complete and tested support already?)
>- Investigation of some rare core dumps. For example, SDLVideo.videoDriverName()
>always crashes on my machine (windows, directx).
>- Support SDL_sound library
>
>Patric
>
>
>
>Ivan Z. Ganza schrieb:
>
>
>>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
>>>
>>>
>>>
>>>
>>-------------------------------------------------------------------------
>>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
>>
>>
>>
>>
>
>-------------------------------------------------------------------------
>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
>
>
|
