Skip to content

Fix context leak in LocationProviderImpl.kt#690

Merged
Chaoba merged 2 commits intomainfrom
kl-location-context
Sep 29, 2021
Merged

Fix context leak in LocationProviderImpl.kt#690
Chaoba merged 2 commits intomainfrom
kl-location-context

Conversation

@Chaoba
Copy link
Copy Markdown
Contributor

@Chaoba Chaoba commented Sep 29, 2021

PRs must be submitted under the terms of our Contributor License Agreement CLA.
Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Apply changelog label ('breaking change', 'bug 🪲', 'build', 'docs', 'feature 🍏', 'performance ⚡', 'testing 💯') or use the label 'skip changelog'
  • Add an entry inside this element for inclusion in the mapbox-maps-android changelog: <changelog>Fix context leak in LocationProviderImpl.kt</changelog>.

Summary of changes

This pr uses WeakReference to wrap context instance to prevent context leak.

User impact (optional)

@Chaoba Chaoba added the bug 🪲 Something isn't working label Sep 29, 2021
@Chaoba Chaoba requested a review from a team September 29, 2021 05:07
@Chaoba Chaoba self-assigned this Sep 29, 2021
Copy link
Copy Markdown
Contributor

@kiryldz kiryldz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit

@Chaoba Chaoba merged commit 5dc1fa0 into main Sep 29, 2021
@Chaoba Chaoba deleted the kl-location-context branch September 29, 2021 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🪲 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants