Skip to content
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

Release FabricMeshes stored in TileLoadThreadResult #414

Merged
merged 2 commits into from
Aug 3, 2023
Merged

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Aug 2, 2023

If a tile made it through prepareInLoadThread but not prepareInMainThread its fabric meshes would not be released back into the pool, causing a memory leak.

The bug was introduced in #389 - fabric meshes were added to the TileLoadThreadResult struct but they were not being released in free.

You can see the fix in action by adding a tileset, removing the tileset, and looking at the pool capacity. Previously the pool capacity would not return back to 2048.

Before After
before-tileset-destroyed after-tileset-destroyed

Copy link
Contributor

@mattelser mattelser left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from update-cesium-native to main August 2, 2023 22:17
Copy link
Contributor

@weegeekps weegeekps left a comment

Choose a reason for hiding this comment

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

LGTM

@lilleyse lilleyse merged commit e9e3abe into main Aug 3, 2023
3 checks passed
@lilleyse lilleyse deleted the fix-memory-leak branch August 3, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants