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: Patric R. <pa...@ma...> - 2006-09-22 00:15:59
|
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
>
>
|
