fix: (darwin) share CVMetalTextureCache to stop per-texture IOSurface leak (#178) - #202
Merged
Merged
Conversation
…ace leak (#178) MetalTextureWrapper.allocate() created a fresh CVMetalTextureCache per texture. Releasing that cache does not synchronously free the IOSurfaces it has cached, so every CVMetalTextureCacheCreateTextureFromImage stranded ~one IOSurface — roughly one screen-sized surface per mount/unmount. Fix: use one process-wide CVMetalTextureCache (created lazily, keyed off the system default device) instead of one per wrapper, and flush it on teardown with kCVMetalTextureCacheMaximumTextureAgeKey: 0. Verified in a standalone reproducer; see docs/DARWIN_IOSURFACE_LEAK_ROOT_CAUSE.md. The darwin texture teardown is otherwise correct (create/destroy of the descriptor, Flutter adapter, and Filament render target are all balanced); the remaining phys_footprint movement is bounded Metal/Filament allocator churn that plateaus, not a leak. No texture pool is required — the pool on the wip branch only masked the async reclamation by reusing one texture. Accordingly rewrite darwin_texture_leak_test.dart to assert bounded (not linear) growth: the steady-state per-session phys_footprint delta over the trailing sessions must stay under one BGRA surface. The original per-cache leak (~one surface/session) trips this; the fix's churn plateau passes with margin. Verified the test catches the regression: steady-state 1.87 MB/session without the fix (FAIL) vs 0.60 MB/session with it (PASS); one surface = 1.69 MB. Co-Authored-By: Claude <noreply@anthropic.com>
nmfisher
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the IOSurface leak (#178).
Root cause was
MetalTextureWrapper.allocate()creating a freshCVMetalTextureCacheper texture. Releasing that cache does not synchronously free the IOSurfaces it has cached, so everyCVMetalTextureCacheCreateTextureFromImagestranded one IOSurface.Fix is to use one process-wide cache (created lazily, keyed off the system default device), flushed on teardown with
kCVMetalTextureCacheMaximumTextureAgeKey: 0. by sharing one process-wideCVMetalTextureCacheinstead of creating one perMetalTextureWrapper.