Skip to content

Fix memory leak in renderer destroy#657

Merged
kiryldz merged 4 commits intomainfrom
kdz-fix-renderer-leak
Sep 22, 2021
Merged

Fix memory leak in renderer destroy#657
kiryldz merged 4 commits intomainfrom
kdz-fix-renderer-leak

Conversation

@kiryldz
Copy link
Copy Markdown
Contributor

@kiryldz kiryldz commented Sep 22, 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.
  • 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 memory leak in renderer destroy.</changelog>.

Summary of changes

Fix memory leak in renderer destroy.
Previously in surface mode we did not call releasing native renderer as Activity.onDestroy call is happening after Android system was sending SurfaceHolder.Callback.surfaceDestroyed when we have already stopped render thread.

Diagram from the profiler after the fix and several opening / closing activity with MapView

image

User impact (optional)

@kiryldz kiryldz added the bug 🪲 Something isn't working label Sep 22, 2021
@kiryldz kiryldz requested a review from a team September 22, 2021 09:27
@kiryldz kiryldz self-assigned this Sep 22, 2021
Comment thread sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt Outdated
Comment thread sdk/src/main/java/com/mapbox/maps/renderer/WorkerHandlerThread.kt Outdated
@kiryldz kiryldz force-pushed the kdz-fix-renderer-leak branch 2 times, most recently from ef1594d to 74db03a Compare September 22, 2021 10:38
@kiryldz kiryldz force-pushed the kdz-fix-renderer-leak branch from 74db03a to f12ae7e Compare September 22, 2021 10:43
Comment thread sdk/src/main/java/com/mapbox/maps/renderer/WorkerHandlerThread.kt Outdated
@kiryldz kiryldz requested a review from Chaoba September 22, 2021 12:11
@kiryldz kiryldz force-pushed the kdz-fix-renderer-leak branch from d0cd6e3 to 0059742 Compare September 22, 2021 13:45
@kiryldz kiryldz merged commit 926c517 into main Sep 22, 2021
@kiryldz kiryldz deleted the kdz-fix-renderer-leak branch September 22, 2021 14:20
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.

6 participants