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

get rid of mb2's .forget() invocations #800

Closed
ctm opened this issue Jan 6, 2022 · 1 comment
Closed

get rid of mb2's .forget() invocations #800

ctm opened this issue Jan 6, 2022 · 1 comment
Assignees
Labels
bug Something isn't working easy Trivial to do (even when tired!) and semi-worthwhile high priority Should be done fairly soon

Comments

@ctm
Copy link
Owner

ctm commented Jan 6, 2022

Get rid of all the calls to .forget() that mb2's web client uses.

I didn't think that calling .forget on a Timeout leaks memory, but it does, at least until we can invoke wasm-bindgen with --weak-refs. There's currently no way to tell web-pack to invoke wasm-bindgen that way though.

I believe we're currently only calling .forget() for resize handlers (i.e. one per window, so that leakage doesn't really matter) and Timeouts. It's the Timeouts that could be an issue, but it's trivial to fix for the resize handlers, since there's an obvious place to store the closure. Each of the Timeouts will require a bespoke solution.

@ctm ctm added bug Something isn't working high priority Should be done fairly soon easy Trivial to do (even when tired!) and semi-worthwhile labels Jan 6, 2022
@ctm ctm self-assigned this Jan 6, 2022
@ctm
Copy link
Owner Author

ctm commented Jan 6, 2022

Done. Deploying now.

@ctm ctm closed this as completed Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working easy Trivial to do (even when tired!) and semi-worthwhile high priority Should be done fairly soon
Projects
None yet
Development

No branches or pull requests

1 participant