Bug 14836 - Memory leak in Surface zoom function
: Memory leak in Surface zoom function
Status: CLOSED FIXED
Product: SqueezePlay
Classification: Unclassified
Component: API
: 7.4.x
: PC Other
: P1 major with 1 vote (vote)
: 7.5.0
Assigned To: Wadzinski Tom
http://forums.slimdevices.com/showthr...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-10-19 09:37 UTC by Erland Isaksson
Modified: 2010-04-08 17:26 UTC (History)
3 users (show)

See Also:
Category: Bug


Attachments
MemLeak applet that can be used to reproduce bug (26.63 KB, application/x-zip)
2009-10-19 09:37 UTC, Erland Isaksson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erland Isaksson 2009-10-19 09:37:36 UTC
Created attachment 6169 [details]
MemLeak applet that can be used to reproduce bug

There seems to be a memory leak in the zoom function.

With code like this it leaks:
local tmp = self:_loadImage("album.png"):zoom(zoomx,zoomy,1)
tmp:blit(screen,positionx,positiony)
tmp:release()

With code like this it doesn't leak:
local tmp = self:_loadImage("album.png")
tmp:blit(screen,positionx,positiony)

The problem can be reproduced with the attached applet "MemLeak", just install
it, configure "Mem Leak" applet as screen saver and let it run for a 15-30
minutes or something similar and you will see that the amount if free memory is
decreasing. It's not decreasing a lot, but it's enough to get out of memory in
a day or two for a screen saver applet that runs continuously.

I found the problem during development on the Album Flow applet which causes
reboots at least once a day due to this bug. I'm not sure the problem is
reproducible on a PC but it's definitely reproducible when executed on a real
Squeezebox Touch device.

I've tried manually calling collectgarbage() but it doesn't help, it still
leaks resources.

It's a problem for all applets that regularly call the Surface:zoom() function.
Comment 1 Erland Isaksson 2009-10-20 09:57:14 UTC
The attached MemLeak applet has now been running 24 hours on my Touch.
The free memory has gone from 65MB to 25MB during this time, so it seems to leak approximately 40MB per 24 hours. 

The Touch has just been standing there with off screen saver active, I haven't played any music or done anything else with the Touch during these 24 hours.
Comment 2 SVN Bot 2009-10-27 12:17:37 UTC
 == Auto-comment from SVN commit #7947 to the jive repo by tom ==
 == https://svn.slimdevices.com/jive?view=revision&revision=7947 ==

Fixed Bug: 14836 +3
Description:
- release() was incorrectly lowering the ref count, which caused srf to never be freed.
Comment 3 Erland Isaksson 2009-10-31 22:29:43 UTC
This doesn't work, in 7965 this leaks more than ever. Instead of surviving two days it now survives 1 minute with the attached MemLeak applet.
Comment 4 Stefan Hansel 2009-11-01 06:30:21 UTC
erland, just a wild guess ...
Could it be, that the original image needs to be released too ?

Like

local tmp1=self:_loadImage(...)
local tmp2=tmp1.zoom(...)
tmp2.blit(...)
tmp2.release()
tmp1.release()
Comment 5 Erland Isaksson 2009-11-01 19:58:35 UTC
(In reply to comment #4)
> erland, just a wild guess ...
> Could it be, that the original image needs to be released too ?
> 
> Like
> 
> local tmp1=self:_loadImage(...)
> local tmp2=tmp1.zoom(...)
> tmp2.blit(...)
> tmp2.release()
> tmp1.release()

The implementation of _loadImage returns the same instance every time, it loads it the first time and return it from the cache when called the next time. So unless I'm missing something I shouldn't have to release a image I plan to use later. zoom() needs the release() call since it returns a new image every time.
Comment 6 SVN Bot 2009-11-02 09:17:14 UTC
 == Auto-comment from SVN commit #7986 to the jive repo by tom ==
 == https://svn.slimdevices.com/jive?view=revision&revision=7986 ==

Bug: 14836 +3
Description:
- release() ignores refcount now, null protection added
Comment 7 Wadzinski Tom 2009-11-02 09:18:22 UTC
Erland, please try again with r7986.
Comment 8 SVN Bot 2009-11-02 10:09:47 UTC
 == Auto-comment from SVN commit #7988 to the jive repo by tom ==
 == https://svn.slimdevices.com/jive?view=revision&revision=7988 ==

Bug: 14836
Description:
- release() ignores refcount now, null protection added
Comment 9 SVN Bot 2009-11-02 10:51:21 UTC
 == Auto-comment from SVN commit #7989 to the jive repo by tom ==
 == https://svn.slimdevices.com/jive?view=revision&revision=7989 ==

Bug: 14836
Description:
- some null protections were unneeded, caused verbose output.
Comment 10 Erland Isaksson 2009-11-04 21:18:00 UTC
(In reply to comment #7)
> Erland, please try again with r7986.

I've now used Album Flow applet for about 8 hours on firmware 7998 and it doesn't seem to leak anymore. It would have leaked at least 10-20 MB with the 7.4 release. So I think we safely can consider this problem solved with your latest fix.
Comment 11 Chris Owens 2010-04-08 17:26:51 UTC
This bug has been marked fixed in a released version of Squeezebox Server or the accompanying firmware or mysqueezebox.com release.

If you are still seeing this issue, please let us know!