Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using container.NewThemeOverride leaks memory until window closing #5000

Closed
2 tasks done
dweymouth opened this issue Jul 15, 2024 · 5 comments
Closed
2 tasks done

Using container.NewThemeOverride leaks memory until window closing #5000

dweymouth opened this issue Jul 15, 2024 · 5 comments
Labels
blocker Items that would block a forthcoming release bug Something isn't working optimization Tickets that could help Fyne apps run faster

Comments

@dweymouth
Copy link
Contributor

dweymouth commented Jul 15, 2024

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

When adding a widget to a container.NewThemeOverride, the widget and its override theme are stored in the overrides map. This is never cleaned out except when a window is closing. This is because the deletion of the widget's entry in the overrides map happens in cache.DestroyRenderer, but that function is only called when a window is closing. Elsewhere (eg in the regular clean task), renderers are directly destroyed instead of by calling this function, which misses the deletion of the entry from the overrides map, if that widget had a theme override.

How to reproduce

just check the code as described above. Or create an app that cycles through new widgets, applying an override to them, and then not using them. It will leak memory

Screenshots

No response

Example code

n/a

Fyne version

2.5.0

Go compiler version

n/a

Operating system and version

n/a

Additional Information

No response

@dweymouth dweymouth added bug Something isn't working optimization Tickets that could help Fyne apps run faster labels Jul 15, 2024
@dweymouth dweymouth added this to the E fixes (v2.5.x) milestone Jul 15, 2024
@Jacalz
Copy link
Member

Jacalz commented Jul 15, 2024

I would potentially consider this a release blocker

@dweymouth
Copy link
Contributor Author

I would potentially consider this a release blocker

Well we already released... but I have a PR for the fix. It is not a big leak in most apps since I imagine most don't create and discard a lot of widgets with overridden themes. We've had much worse leaks in releases before that no one (except me) complained about ;) e.g. when we were leaking every image.Image that had ever been rendered in a canvas.Image

@Jacalz
Copy link
Member

Jacalz commented Jul 15, 2024

Well we already released...

I know we released 2.5.0, I was talking about the first bug fix release.

@dweymouth
Copy link
Contributor Author

Well we already released...

I know we released 2.5.0, I was talking about the first bug fix release.

Ah, yeah makes sense, I'll add the blocker label

@dweymouth dweymouth added the blocker Items that would block a forthcoming release label Jul 15, 2024
@andydotxyz
Copy link
Member

This looks to be fixed then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Items that would block a forthcoming release bug Something isn't working optimization Tickets that could help Fyne apps run faster
Projects
None yet
Development

No branches or pull requests

3 participants