-
Notifications
You must be signed in to change notification settings - Fork 4k
fix(database): refactor "removeObserver" to fix memory leak #2717
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
@invertase This PR, along with a followup for a similar implementation on |
Thanks @ened I'll take a look. |
@kroikie do you have any update or thoughts on this? |
@Ehesp - FYI - guess this becomes obselete once the rework is ready? |
@ened is this PR still valid, if so I'd like to get it reviewed and shipped - let me know, thanks :) |
…erfire into firebase_database_memory_leak
@Salakar still relevant, yes. |
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.
LGTM, thank you again for another PR :)
Agree, we're still tweaking the new CI setup on GitHub actions but I think once we're comfortable everything is running smoothly, de-flaked and stable we can start introducing new checks like this and other things like pod lint and Android lint build checks too. |
Description
Similar to #261, this PR removes a memory leak when a App is analyzed using the LeakCanary plugin.
The Android lifecycle is very strict when the "Don't Keep Activities" developer setting is turned on. In that case, only the native side of the plugin will get a callback to release the resources. This has been partially mentioned in flutter/flutter#32948.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?