sdljava-users

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

[Jesper J. / S. G. <je...@so...>]

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

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

[Ivan Z. G. <iva...@ya...>]

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
>  
>

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
> 
> 

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

[Ivan Z. G. <iva...@ya...>]

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
>  
>

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

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

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
> 
> 

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

[Ivan Z. G. <iva...@ya...>]

HI Patric,

I will fix all instances, thanks for pointing those out.  I plan to do 
the fixes tomorrow at some point.

Thanks for the wishlist, I agree we should find a way to get all that in.

I remember adding code for YUV but I'm not really sure what YUV does so 
I don't know how to test it properly.

The core dump problem on videoDriverName baffled me for a while, then I 
gave up to go to more important things, its quite strange.

Regarding SDL_Sound I wasn't sure if people actually use it?  Are people 
using this?  And why would you use that instead of the Mixer?

For Windows we just need to get into CVS the scripts that are needed to 
build the various binaries.  Most of that is there already -- so there 
shouldn't be too much work.  When we have a release someone needs to be 
so kind as to build me the binaries, and send them to me so I can add 
them to the release archive.

-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
>  
>

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

[Ivan Z. G. <iva...@ya...>]

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
>  
>