-
Notifications
You must be signed in to change notification settings - Fork 140
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
fix: Prevent a memory leak on AndroidView which may not call ON_DESTROY #188
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
I signed it. |
@arriolac I see the test problems have been fixed - can we re-run this and merge unless there is no problem? |
This should be good to merge. Will let @wangela take over from here |
Thank you for your contribution @polivmi1 ! |
🎉 This PR is included in version 2.7.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
So sorry @polivmi1, would you mind
and then push the branch again? The reason for this is the original commit message did not follow conventional commit conventions, and was interpreted by our semantic release bot as not triggering a new release when in fact it is a bugfix and should trigger a patch release. I have reverted the merge from this PR so you can re-submit your changes. Once you push the branch, it will create a new PR and you can reference this PR #188 and the original issue that is fixed #138 in the new PR's description, to link to the historical discussion preserved in this PR review. Apologies for not catching the commit message format during the main review. |
@wangela I have changed the commit message in https://github.com/polivmi1/android-maps-compose/tree/138_memory_leak it hasn't created a new PR (this is already in state merged). Should I create a completely new PR and mention this one there? |
@polivmi1 Yes, I see a "Create pull request" button here that you can use: main...polivmi1:android-maps-compose:138_memory_leak |
Waiting for this release to be fixed |
This PR fixes a memory leak that can happen in AndroidView when navigating to another screen.
The Lifecycle ON_DESTROY event is never called because in onDispose we remove the mapLifecycleObserver and this method is called before the Lifecycle ON_DESTROY event.
We have to handle any Lifecycle ON_DESTROY in the onDispose method.
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #138 🦕