-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: Prevent GutenbergKit retain cycle #24936
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
Conversation
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 29562 | |
Version | PR #24936 | |
Bundle ID | com.jetpack.alpha | |
Commit | 35a0f0c | |
Installation URL | 1j37can62algg |
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 29562 | |
Version | PR #24936 | |
Bundle ID | org.wordpress.alpha | |
Commit | 35a0f0c | |
Installation URL | 4reufcgd6eaf0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a memory leak in GutenbergKit by preventing a retain cycle that was preventing proper disposal of WebViews. The fix addresses strong reference cycles in the Autosaver and improves cleanup of view controllers and tasks during deinitialization.
- Changed Autosaver closure to use weak self reference to break retain cycle
- Added proper cleanup of loading tasks and child view controllers in deinit
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
WordPress/Classes/ViewRelated/NewGutenberg/NewGutenbergViewController.swift
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/NewGutenberg/NewGutenbergViewController.swift
Outdated
Show resolved
Hide resolved
The memory leak likely leads to performance degradations and crashes.
This is effectively a no-op as it already occurs via UIKit behavior.
Allow clean up while the editor closes.
d7071d7
to
35a0f0c
Compare
|
Approving the "autosaver" part, but I've noticed a couple of issues with loading dependencies. For example, it cancels the loading when you open "Revisions", and it also shows an infinite spinner when the dependencies fail load. I'll open a PR addressing these. |
Description
Close CMM-834.
It appears the strong
Autosaver
references and, possibly, lack of cleaning up asset loading tasks and view elements led to a retain cycle. The retain cycle prevented proper disposal of WebViews. The memory leak likely leads to performance degradations and crashes.Testing instructions
1. Verify pending changes are persisted via autosave mechanism
2. Verify WebViews are disposed upon editor close