<<Fix for memory leak>>> every dev read

[whise]

the title says it all ... after alot of testing i found out that the text drawing using ctx causes a memory because the ctx memory isnt cleared , we got to fix it
you will only ontice it on screenlets that update very quickly
here is why is appends

traditionaly python has problems freeing memory it uses because it normally store vars forever , here's what appends in the text memory leak
in def on_draw
p_layout = ctx.create_layout()

in here python stores the p_layout , the next time it redraws it stores it again and again and again , thats why we get a memory leak

how do we fix it , simple

first provide a global var

class NetmonitorScreenlet(screenlets.Screenlet):

p_layout = None

then in on_draw

if self.p_layout == None :

self.p_layout = ctx.create_layout()
else:

ctx.update_layout(self.p_layout)

thats it , now we just update the var instead of creating a new one ;)

[whise]

i think everyone should update their screenlets like this ...

[James]

My screenlet was having this exact memory problem.

I've tried your fix and I can confirm it does work.

Good work on the find!

-- James

[cbudden]

Is there a list of screenlets that have been updated to include this fix?

Chris

[whise]

actually none except mine , i guess people will start to use this now , and also you wont really notice it unless the screenlet updates really frequently

[RYX]

Thanks for tracing it down, whise - good work!

You can also create the PangoLayout inside the constructor (be getting it from self.window.window). I already did the same for the ClockScreenlet before the last release, I guess that's why the Clock doesn't have the leak ...

How about another attribute to the constructor to (optionally) automatically create self.p_layout? Would remove the need for creating the layout in all screenlets.

:)

[whise]

i guess that would be a good idea , would improve the development easiness and would also stop people from making that error , also i hope it wont break backwards compatibility

[whise]

could this be a sticky please

[jsf]

Thanks immensely for this tip, whise. I was wondering where all the memory was going and dreading tracking it down. This information needs to be un-missable for developers of 3rd party screenlets. What about just adding this to the main "Infos for writing/releasing own screenlets" thread? Or even to the documentation section on the screenlets.org website?

[whise]

thats what i think too

[nomego]

Isn't it just enough to do a p_layout = None after ctx.restore() or even ctx.show_layout() ?

[davim]

I think not because you wont be freeing the memory, just pointing to another place....

[nomego]

Could anyone verify I made it right in my Terminals screenlet (http://screenlets.org/index.php/Terminals) so I know that I'll modify my other screenlets correctly?

[whise]

i checked your code its correct , RYX already knows about this issue , im thinking a Draw_text function could be integrated into the core so it could be used by developers an not mislead them

[nomego]

Ok I will update my other screenlets accordingly.

[taril]

This is not enough!

You have to search for all the p_layout values in the file, and you have to insert before it the "SELF.", so it has to look like this:

self.p_layout

[aantn]

Whise, there seems to be a memory leak in the functions theme.get_text_width and theme.get_text_extents.

I'm going to fix it in my branch, and you should do so in trunk.

As a side note, I'll give you advance warning that I'm making a bunch of the theme methods into static methods. The advantage is that they can now be used by classes that don't have their own theme. (Right now, that's only applicable in my branch.)

i was thinking about doing that for some time now ... thanks

[aantn]

Actually, I don't think it's really a memory leak. It's not the most efficient way of handling things, but it shouldn't cause leakage.

I'll add on an optional parameter in case the pango layout was already created. It should help in many cases.

[whise]

thanks i didnt notice it,
about the themes , i wass thinking about that for a long time , good job

[Ashy]

Yes, move those methods, silly having them in theme and has caused me a few headaches :)

[aantn]

I'm running into some annoyances by keeping them in the Screenlet class.

Do you have any objection to make them module functions, or at the very least static class functions?

[whise]

do you mean keeping them on the screenlettheme class?

i dont mind , but we must think of a better solution , maybe rethink the whole screenlettheme class

[aantn]

You're right. Perhaps it's time to merge SreenletTheme into Screenlet and move the drawing functions into their own module so that they can be shared and used in other places.

[aantn]

Ignore my last post. After the conversation about global themes, I've changed my mind. Let's leave the ScreenletTheme class in a separate module.

[mrthefter]

The countdown screenlet still appears to have this problem.

[aantn]

I'll give it a look later.

I haven't quite gotten rid of the ScreenletTheme class, but I've rewritten most of the theming code so that themes can now be used outside of Screenlets. It's past time to take over the desktop. :-D