[Impeller] Dispose thread local caches on each frame when using GPUSurfaceVulkanImpeller with a delegate#182265
Merged
auto-submit[bot] merged 3 commits intoflutter:masterfrom Feb 13, 2026
Conversation
Member
Author
|
Noticed this memory leak while looking at #181966 / #181967 cc @RickyvdBerg - if you are using the Impeller/Vulkan back end with the embedder and |
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses a potential resource leak in the Impeller Vulkan backend by ensuring that thread-local caches are disposed on each frame when GPUSurfaceVulkanImpeller is used with a delegate. The security audit could not identify any vulnerabilities as the provided files were empty, preventing a meaningful analysis. Overall, the changes are logical and well-implemented, including the correct addition of a call to DisposeThreadLocalCachedResources and the introduction of a new unit test to verify this behavior.
Comment on lines
+62
to
+76
| auto context = impeller::ContextVK::Create(std::move(context_settings)); | ||
|
|
||
| TestGPUSurfaceVulkanDelegate delegate; | ||
|
|
||
| std::unique_ptr<Surface> surface = | ||
| std::make_unique<GPUSurfaceVulkanImpeller>(&delegate, context); | ||
|
|
||
| // Add a command pool to the global map. | ||
| auto pool = context->GetCommandPoolRecycler()->Get(); | ||
| EXPECT_EQ(impeller::CommandPoolRecyclerVK::GetGlobalPoolCount(*context), 1); | ||
|
|
||
| // Check that AcquireFrame disposes thread local resources and removes | ||
| // the pool from the global map. | ||
| auto frame = surface->AcquireFrame(DlISize(100, 100)); | ||
| EXPECT_EQ(impeller::CommandPoolRecyclerVK::GetGlobalPoolCount(*context), 0); |
Contributor
There was a problem hiding this comment.
The test would be more robust if it included assertions to verify that the objects (context, surface, pool, frame) are created successfully before they are used. This ensures that the test fails early and clearly if setup fails, rather than potentially crashing or failing with a less specific error later on.
auto context = impeller::ContextVK::Create(std::move(context_settings));
ASSERT_TRUE(context && context->IsValid());
TestGPUSurfaceVulkanDelegate delegate;
std::unique_ptr<Surface> surface =
std::make_unique<GPUSurfaceVulkanImpeller>(&delegate, context);
ASSERT_TRUE(surface->IsValid());
// Add a command pool to the global map.
auto pool = context->GetCommandPoolRecycler()->Get();
ASSERT_NE(pool, nullptr);
EXPECT_EQ(impeller::CommandPoolRecyclerVK::GetGlobalPoolCount(*context), 1);
// Check that AcquireFrame disposes thread local resources and removes
// the pool from the global map.
auto frame = surface->AcquireFrame(DlISize(100, 100));
ASSERT_NE(frame, nullptr);
EXPECT_EQ(impeller::CommandPoolRecyclerVK::GetGlobalPoolCount(*context), 0);
gaaclarke
requested changes
Feb 11, 2026
Member
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Code looks good and test looks good. I'd just like to see this incorporated as part of impeller_unittests instead of adding yet another executable.
| } | ||
| } | ||
|
|
||
| if (impeller_enable_vulkan) { |
Member
There was a problem hiding this comment.
Please add these tests as part of impeller_unittests instead of making a new executable.
Member
Author
There was a problem hiding this comment.
Impeller is mostly independent of the rest of Flutter.
I'd prefer to avoid having the impeller_unittests binary incorporate code from the flutter/shell/gpu tree.
This PR adds a shell/gpu test binary for Vulkan similar to the existing one for Metal.
Member
There was a problem hiding this comment.
Okay, not adding it to impeller_unittests sounds good. Can we add this test to the existing metal one then?
Member
Author
There was a problem hiding this comment.
gpu_surface_metal_unittests in https://github.com/flutter/flutter/blob/master/engine/src/flutter/shell/gpu/BUILD.gn is specific to Apple platforms.
The Vulkan test should be buildable on the platforms that support Vulkan/SwiftShader.
Member
There was a problem hiding this comment.
Right, I'm thinking we do like what we do in impeller_unittests where we only link in tests that are applicable to the testing platform. So the executable will be gpu_surface_unittests, if you are building for macos you will build and link in the metal tests, on linux you won't. We can have TEST_P tests if we want the same test across multiple backens.
Member
Author
There was a problem hiding this comment.
Merged the shell/gpu test binaries - PTAL
gaaclarke
approved these changes
Feb 12, 2026
Member
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Thanks jason! Our testing setup for the engine is pretty complicated, I'm operating under the theory that the less testing executables there are the easier it is for people to find and execute tests. When there is separate binaries too it's easier for them to drift too. Hopefully his helps.
…rfaceVulkanImpeller with a delegate The Impeller Vulkan back end caches command pools in thread-local storage. These caches can grow unbounded unless the embedder calls ContextVK::DisposeThreadLocalCachedResources() If the GPUSurfaceVulkanImpeller does not have a delegate, then AcquireFrame will invoke DisposeThreadLocalCachedResources through its call to SurfaceContextVK::AcquireNextSurface. With a delegate, AcquireFrame follows a different code path that must explicitly call DisposeThreadLocalCachedResources.
jason-simmons
force-pushed
the
vulkan_delegate_thread_leak
branch
from
1102705 to
b572a37
Compare
jason-simmons
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 13, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 14, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 14, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 14, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 14, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 15, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 15, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 15, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 16, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 16, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 16, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 17, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 17, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 17, 2026
…sing GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265)
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Feb 17, 2026
Roll Flutter from 9bda20a11f1e to 6e4a481bdf27 (103 revisions) flutter/flutter@9bda20a...6e4a481 2026-02-17 [email protected] Fix iOS CI tests for Xcode 26 Swift compatibility (flutter/flutter#182132) 2026-02-17 [email protected] Revert "[Android] Add mechanism for setting Android engine flags via … (flutter/flutter#182388) 2026-02-17 [email protected] Roll Skia from 4ed9faf843e6 to dfe78d132e24 (1 revision) (flutter/flutter#182485) 2026-02-17 [email protected] Roll Skia from ff0af46bf172 to 4ed9faf843e6 (2 revisions) (flutter/flutter#182483) 2026-02-17 [email protected] Roll Skia from 24c7b6f5760f to ff0af46bf172 (1 revision) (flutter/flutter#182481) 2026-02-16 [email protected] Roll Dart SDK from ff57548fcf54 to 44895e617182 (1 revision) (flutter/flutter#182479) 2026-02-16 [email protected] Roll Fuchsia Linux SDK from YND8TyaxKkkkEvlD9... to mcN42vw48OPH3JDNm... (flutter/flutter#182478) 2026-02-16 [email protected] Roll Dart SDK from c819ebe0cbe3 to ff57548fcf54 (1 revision) (flutter/flutter#182472) 2026-02-16 [email protected] feat: add routes support in TestWidgetsApp (flutter/flutter#181695) 2026-02-16 [email protected] Roll Skia from 5c8a6641902f to 24c7b6f5760f (1 revision) (flutter/flutter#182467) 2026-02-16 [email protected] Roll Skia from 94d5d5e5f785 to 5c8a6641902f (6 revisions) (flutter/flutter#182463) 2026-02-16 [email protected] Roll Dart SDK from f2289e13a20a to c819ebe0cbe3 (1 revision) (flutter/flutter#182462) 2026-02-15 [email protected] Roll Dart SDK from 294e6e248512 to f2289e13a20a (1 revision) (flutter/flutter#182448) 2026-02-15 [email protected] Roll Skia from b7cea4cbe546 to 94d5d5e5f785 (1 revision) (flutter/flutter#182446) 2026-02-15 [email protected] Roll Fuchsia Linux SDK from pkyhAZ3sQZDzeNZym... to YND8TyaxKkkkEvlD9... (flutter/flutter#182445) 2026-02-15 [email protected] Roll Skia from a3a82d359a7b to b7cea4cbe546 (1 revision) (flutter/flutter#182439) 2026-02-15 [email protected] Roll Skia from a147ae2d4adc to a3a82d359a7b (1 revision) (flutter/flutter#182435) 2026-02-15 [email protected] Roll Dart SDK from f82ec89435f5 to 294e6e248512 (1 revision) (flutter/flutter#182432) 2026-02-14 [email protected] Roll Skia from 91d158b0a61e to a147ae2d4adc (2 revisions) (flutter/flutter#182424) 2026-02-14 [email protected] Roll Fuchsia Linux SDK from V30FBkJySjFKXwVjW... to pkyhAZ3sQZDzeNZym... (flutter/flutter#182423) 2026-02-14 [email protected] Roll Skia from e5a18f8f0d4a to 91d158b0a61e (1 revision) (flutter/flutter#182422) 2026-02-14 [email protected] Roll Dart SDK from 7a2a28dbd0d4 to f82ec89435f5 (2 revisions) (flutter/flutter#182414) 2026-02-14 [email protected] Roll Skia from befeec673f1b to e5a18f8f0d4a (1 revision) (flutter/flutter#182412) 2026-02-14 [email protected] Roll Skia from 7dc3ba9b1d90 to befeec673f1b (1 revision) (flutter/flutter#182408) 2026-02-14 [email protected] [Web] Fix IME and selection by syncing more text styles (flutter/flutter#180436) 2026-02-14 [email protected] Roll Skia from bb69b5b71b4f to 7dc3ba9b1d90 (2 revisions) (flutter/flutter#182401) 2026-02-14 [email protected] Disable multithread opengles, enables remaining fragment shader tests (flutter/flutter#182384) 2026-02-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Standardize on Test* widgets in *_tester.dart files (#182395)" (flutter/flutter#182406) 2026-02-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix cross imports for all Cupertino tests (#181634)" (flutter/flutter#182404) 2026-02-13 [email protected] Add await to tester.pump callsites (flutter/flutter#182398) 2026-02-13 [email protected] refactor: Centralize table formatting logic into a new `formatTable` utility function. (flutter/flutter#182196) 2026-02-13 [email protected] Adds impeller backend to golden workspace name (flutter/flutter#182387) 2026-02-13 [email protected] Standardize on Test* widgets in *_tester.dart files (flutter/flutter#182395) 2026-02-13 [email protected] Remove Material dependency from transformed_scrollable_test.dart (flutter/flutter#182141) 2026-02-13 [email protected] Fix cross imports for all Cupertino tests (flutter/flutter#181634) 2026-02-13 [email protected] remove MaterialApp import from raw_radio_test.dart (flutter/flutter#181721) 2026-02-13 [email protected] Roll Skia from e2991aa99710 to bb69b5b71b4f (37 revisions) (flutter/flutter#182390) 2026-02-13 [email protected] Turns on most of fragment_shader_test.dart for opengles (flutter/flutter#182229) 2026-02-13 [email protected] Update `CHANGELOG` for 3.41.1 release (flutter/flutter#182393) 2026-02-13 [email protected] Remove Material dependency from semantics_keep_alive_offstage_test.dart (flutter/flutter#182211) 2026-02-13 [email protected] Update iOS/macOS plugin template to add dependency on FlutterFramework (flutter/flutter#181416) 2026-02-13 [email protected] Add plugin dependencies to Add to App FlutterPluginRegistrant (flutter/flutter#182304) 2026-02-13 [email protected] Preparation to add contentTextStyle flag to SimpleDialog. (flutter/flutter#182200) 2026-02-13 [email protected] Roll Packages from af1d610 to 09104b0 (4 revisions) (flutter/flutter#182383) 2026-02-13 [email protected] Roll Dart SDK from de5915148fde to 7a2a28dbd0d4 (2 revisions) (flutter/flutter#182380) 2026-02-13 [email protected] [Impeller] Dispose thread local caches on each frame when using GPUSurfaceVulkanImpeller with a delegate (flutter/flutter#182265) ...
rickhohler
pushed a commit
to rickhohler/flutter
that referenced
this pull request
Feb 19, 2026
…rfaceVulkanImpeller with a delegate (flutter#182265) The Impeller Vulkan back end caches command pools in thread-local storage. These caches can grow unbounded unless the embedder calls ContextVK::DisposeThreadLocalCachedResources() If the GPUSurfaceVulkanImpeller does not have a delegate, then AcquireFrame will invoke DisposeThreadLocalCachedResources through its call to SurfaceContextVK::AcquireNextSurface. With a delegate, AcquireFrame follows a different code path that must explicitly call DisposeThreadLocalCachedResources.
RickyvdBerg
added a commit
to RickyvdBerg/flutter
that referenced
this pull request
Mar 5, 2026
…path The Impeller Vulkan backend caches command pools and descriptor pools in thread-local storage. These caches grow unbounded unless DisposeThreadLocalCachedResources() is called each frame. PRs flutter#182265 and flutter#182402 fixed this for GPUSurfaceVulkanImpeller (delegate path) and libImpeller (C API) respectively, but the Flutter Embedder API compositor path through EmbedderExternalViewEmbedder was missed. EmbedderExternalViewEmbedder::SubmitFlutterView calls builder.Render() which calls EmbedderExternalView::Render() which calls the RenderToTarget overload that does not dispose cached resources. This causes linear memory growth from leaked VkCommandBuffers (~20-40KB each) and VkDescriptorPools (~40-80KB each, one per ~80 frames). Add DisposeThreadLocalCachedResources() after builder.Render() in SubmitFlutterView, matching the pattern used in the other two paths. The call goes through the base impeller::Context interface so it is a no-op on non-Vulkan backends. Related to flutter#182265 Related to flutter#182402
10 tasks
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
2 participants
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.
The Impeller Vulkan back end caches command pools in thread-local storage. These caches can grow unbounded unless the embedder calls ContextVK::DisposeThreadLocalCachedResources()
If the GPUSurfaceVulkanImpeller does not have a delegate, then AcquireFrame will invoke DisposeThreadLocalCachedResources through its call to SurfaceContextVK::AcquireNextSurface.
With a delegate, AcquireFrame follows a different code path that must explicitly call DisposeThreadLocalCachedResources.