Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size #175450

jason-simmons · 2025-09-16T19:35:41Z

Also add locks around some methods that access texture and drawable state.

gemini-code-assist

Code Review

This pull request aims to prevent crashes when presenting textures after a drawable size change by adding a size check. It also enhances thread safety by adding locks around methods that access shared texture and drawable state. While the size check in presentTexture: should prevent the immediate crash, it introduces a subtle but significant issue where stale, incorrectly-sized textures are returned to the texture pool, leading to resource pollution. My review includes a high-severity comment detailing this issue in FlutterMetalLayer.mm and a medium-severity comment on the new test in FlutterMetalLayerTest.mm with a suggestion to modify it to expose this underlying bug.

gemini-code-assist · 2025-09-16T19:37:50Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm

+    if (texture.texture.width != _drawableSize.width ||
+        texture.texture.height != _drawableSize.height) {
+      return;
+    }


This check correctly prevents presenting a texture with a mismatched size, which should fix the reported crash. However, by simply returning, a new issue is introduced: the stale texture is eventually returned to the available textures pool.

Here's the sequence of events:

This method returns early.

The FlutterDrawable that holds this texture is not marked as _presented.

When the FlutterDrawable is deallocated, its dealloc method calls returnTexture: because _presented is false.

returnTexture: adds the stale, incorrectly-sized texture to _availableTextures.

This pollutes the texture pool, leading to memory being held by unusable textures and subsequent calls to nextDrawable potentially receiving these stale textures, which will then be rejected again by this check.

The root cause appears to be in setDrawableSize:, where _totalTextures is reset to 0, causing the layer to lose track of in-flight textures. A complete solution would involve correctly managing _totalTextures across resizes and ensuring stale textures are discarded rather than returned to the pool.

Added a texture size check to returnTexture

gemini-code-assist · 2025-09-16T19:37:50Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayerTest.mm

+- (void)testResizeAndPresent {
+  FlutterMetalLayer* layer = [self addMetalLayer];
+  TestCompositor* compositor = [[TestCompositor alloc] initWithLayer:layer];
+
+  id<CAMetalDrawable> oldSizeDrawable1 = [layer nextDrawable];
+  BAIL_IF_NO_DRAWABLE(oldSizeDrawable1);
+  id<CAMetalDrawable> oldSizeDrawable2 = [layer nextDrawable];
+  BAIL_IF_NO_DRAWABLE(oldSizeDrawable2);
+
+  const CGFloat newSize = 200;
+  layer.drawableSize = CGSizeMake(newSize, newSize);
+
+  // After resizing, present the drawables that were allocated using the old size.
+  [oldSizeDrawable1 present];
+  [compositor commitTransaction];
+  [oldSizeDrawable2 present];
+  [compositor commitTransaction];
+
+  // Verify that textures with the old size have been removed from the layer.
+  for (int i = 0; i < 4; i++) {
+    id<CAMetalDrawable> drawable = [layer nextDrawable];
+    [drawable present];
+    [compositor commitTransaction];
+    XCTAssertEqual(drawable.texture.width, newSize);
+  }
+
+  [self removeMetalLayer:layer];
+}


This test case is a great addition for the resize scenario. However, it doesn't currently expose the subtle issue of stale textures being returned to the pool because the oldSizeDrawable objects are held for the entire duration of the method.

If the old drawables were deallocated before the verification loop, nextDrawable would pick up the stale textures from the pool, and the assertion XCTAssertEqual(drawable.texture.width, newSize) would fail. To make this test more robust and catch this bug, you could scope the lifetime of the old drawables so they are deallocated before the verification loop.

For example:

- (void)testResizeAndPresent { FlutterMetalLayer* layer = [self addMetalLayer]; TestCompositor* compositor = [[TestCompositor alloc] initWithLayer:layer]; const CGFloat newSize = 200; { id<CAMetalDrawable> oldSizeDrawable1 = [layer nextDrawable]; BAIL_IF_NO_DRAWABLE(oldSizeDrawable1); id<CAMetalDrawable> oldSizeDrawable2 = [layer nextDrawable]; BAIL_IF_NO_DRAWABLE(oldSizeDrawable2); layer.drawableSize = CGSizeMake(newSize, newSize); // After resizing, present the drawables that were allocated using the old size. [oldSizeDrawable1 present]; [compositor commitTransaction]; [oldSizeDrawable2 present]; [compositor commitTransaction]; } // old drawables are deallocated here // Verify that textures with the old size are not reused. for (int i = 0; i < 4; i++) { id<CAMetalDrawable> drawable = [layer nextDrawable]; [drawable present]; [compositor commitTransaction]; XCTAssertEqual(drawable.texture.width, newSize); } [self removeMetalLayer:layer]; }

With this structure, the test would fail with the current implementation, correctly identifying the texture pool pollution issue.

chinmaygarde · 2025-09-16T20:27:48Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm

-  _totalTextures = 0;
-  _drawableSize = drawableSize;
+  @synchronized(self) {
+    [_availableTextures removeAllObjects];


Can we add a comment near where the ivars _availableTextures, _totalTextures and _front are declared that access to these must be synchronized?

chinmaygarde · 2025-09-16T20:29:18Z

engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm

 - (void)returnTexture:(FlutterTexture*)texture {
  @synchronized(self) {
-    [_availableTextures addObject:texture];
+    if (texture.texture.width == _drawableSize.width &&


Nit: nil check the texture as well? Before the at-synchronized.

chinmaygarde

Some nits. But not presenting or recycling textures of an outdated size makes sense to me. Also, the lack of synchronization seems like an oversight.

Perhaps @knopp can also take another look?

…nged and the texture's size does not match the new drawable size Also add locks around some methods that access texture and drawable state. Fixes flutter#175253

…e size changed and the texture's size does not match the new drawable size (flutter/flutter#175450)

…nged and the texture's size does not match the new drawable size (flutter#175450) Also add locks around some methods that access texture and drawable state. Fixes flutter#175253

…e size changed and the texture's size does not match the new drawable size (flutter/flutter#175450)

Roll Flutter from 4a042046a0f1 to b1a28bc065b0 (44 revisions) flutter/flutter@4a04204...b1a28bc 2025-09-25 [email protected] web_ui: avoid crash for showPerformanceOverlay; log 'not supported' once (flutter/flutter#173518) 2025-09-25 [email protected] Ignore upcoming `experimental_member_use` warnings. (flutter/flutter#175969) 2025-09-25 [email protected] Roll Skia from 753ce2221ce7 to 55436d87e414 (16 revisions) (flutter/flutter#176004) 2025-09-25 [email protected] Add google_fonts to team-framework triage guidelines (flutter/flutter#175675) 2025-09-25 [email protected] Add tests for InputDecoration borders (M3 and theme normalization) (flutter/flutter#175838) 2025-09-24 [email protected] Update Flutter's templates to use dot shorthands (flutter/flutter#175891) 2025-09-24 [email protected] In Gradle Flutter task, correctly replace '\ ' with ' '. (flutter/flutter#175815) 2025-09-24 [email protected] Cleans up navigator pop and remove logic (flutter/flutter#175612) 2025-09-24 [email protected] Fix docs in `EditableText` (flutter/flutter#175787) 2025-09-24 [email protected] Fixes SemanticsFlags.isLink mis-translated in dart ui ffi (flutter/flutter#175812) 2025-09-24 [email protected] Update AGP/Java/Gradle comparison when using analyze --suggestions (flutter/flutter#175808) 2025-09-24 [email protected] Fix SliverMainAxisGroup SliverEnsureSemantics support (flutter/flutter#175671) 2025-09-24 [email protected] Migrate to `WidgetStateColor` (flutter/flutter#175573) 2025-09-24 [email protected] Make sure that a FlexibleSpaceBar doesn't crash in 0x0 environment (flutter/flutter#175228) 2025-09-24 [email protected] Roll Fuchsia Test Scripts from BWj3yYC74ud58QhN0... to APSBP-sS-3FX69Ihf... (flutter/flutter#175944) 2025-09-24 [email protected] Make sure that a MaterialApp doesn't crash in 0x0 environment (flutter/flutter#173090) 2025-09-24 [email protected] feat(cupertino): Add selectableDayPredicate parameter to CupertinoDatePicker for selectable day control #171332 (flutter/flutter#171334) 2025-09-24 [email protected] Refactor `FlutterInjectorTest` to use lambdas/method reference (flutter/flutter#175777) 2025-09-24 [email protected] Replace curly braces with lambdas in `KeyEventChannelTest` (flutter/flutter#175729) 2025-09-24 [email protected] [ Widget Preview ] Fix filter by file on Windows (flutter/flutter#175783) 2025-09-24 [email protected] use lambda expressions /method reference to fix linter issue in `DartMessengerTest.java` (flutter/flutter#175733) 2025-09-24 [email protected] Roll Packages from 3413b65 to 117bf63 (9 revisions) (flutter/flutter#175935) 2025-09-24 [email protected] refactor code to use method reference and lambdas in `DartMessengerTest.java` (flutter/flutter#175731) 2025-09-24 [email protected] Simplify/fix ordering of asserts in `TextInputPluginTest` (flutter/flutter#175784) 2025-09-24 [email protected] Introduce a getter for `Project` to get `gradle-wrapper.properties` directly (flutter/flutter#175485) 2025-09-24 [email protected] Change the arguments order in `assertEquals` to fix linter issues (flutter/flutter#175719) 2025-09-24 [email protected] Broken link in NavigationRail documentation (flutter/flutter#175852) 2025-09-24 [email protected] Updates to flutter web triage links (flutter/flutter#175791) 2025-09-24 [email protected] Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size (flutter/flutter#175450) 2025-09-24 [email protected] Remove comment about trailing commas from templates (flutter/flutter#175864) 2025-09-24 [email protected] Roll Skia from 1c1b19f2ffc3 to 753ce2221ce7 (4 revisions) (flutter/flutter#175909) 2025-09-24 [email protected] Roll Skia from 3191a822cf10 to 1c1b19f2ffc3 (2 revisions) (flutter/flutter#175896) 2025-09-24 [email protected] Roll Skia from cabeab8cb22c to 3191a822cf10 (14 revisions) (flutter/flutter#175894) 2025-09-24 [email protected] Roll Dart SDK from 14b4ced3022a to 899c7340cc4c (4 revisions) (flutter/flutter#175893) 2025-09-24 [email protected] Roll `package:analyzer` forward to `8.2.0`. (flutter/flutter#175849) 2025-09-24 [email protected] Make sure that a VerticalDivider doesn't crash at 0x0 environment (flutter/flutter#174761) 2025-09-24 [email protected] Make sure that Drawer & DrawerHeader don't crash in 0x0 environment (flutter/flutter#174772) 2025-09-24 [email protected] Add an assertion for the relationship between `Visibility.maintainState` and `Visibility.maintainFocusability` (flutter/flutter#175552) 2025-09-24 [email protected] fix: remove final class modifier on MenuController (flutter/flutter#174490) 2025-09-24 [email protected] fix: cupertino sheet broken example with programatic pop (flutter/flutter#175709) 2025-09-24 [email protected] [web] Fix assertion thrown when hot restarting during animation (flutter/flutter#175856) 2025-09-24 [email protected] Add non uniform TableBorder (flutter/flutter#175773) 2025-09-23 [email protected] fix small typo in test docs (flutter/flutter#175776) 2025-09-23 [email protected] Use `assertNull` to simplify code (flutter/flutter#175720) If this roll has caused a breakage, revert this CL and stop the roller ...

…e size changed and the texture's size does not match the new drawable size (flutter/flutter#175450)

…nged and the texture's size does not match the new drawable size (flutter#175450) Also add locks around some methods that access texture and drawable state. Fixes flutter#175253

jason-simmons requested a review from chinmaygarde September 16, 2025 19:35

jason-simmons requested a review from a team as a code owner September 16, 2025 19:35

github-actions bot added platform-ios iOS applications specifically engine flutter/engine related. See also e: labels. team-ios Owned by iOS platform team labels Sep 16, 2025

gemini-code-assist bot reviewed Sep 16, 2025

View reviewed changes

chinmaygarde reviewed Sep 16, 2025

View reviewed changes

chinmaygarde approved these changes Sep 16, 2025

View reviewed changes

jason-simmons added 3 commits September 22, 2025 13:59

Do not present textures in FlutterMetalLayer if the drawable size cha…

d5044ca

…nged and the texture's size does not match the new drawable size Also add locks around some methods that access texture and drawable state. Fixes flutter#175253

returnTexture size check

b8c6c65

feedback

9855aac

jason-simmons force-pushed the bug_175253 branch from 199a525 to 9855aac Compare September 22, 2025 20:59

jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2025

auto-submit bot added this pull request to the merge queue Sep 23, 2025

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 24, 2025

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2025

jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2025

auto-submit bot added this pull request to the merge queue Sep 24, 2025

Merged via the queue into flutter:master with commit 9d20398 Sep 24, 2025
189 of 190 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 24, 2025

engine-flutter-autoroll mentioned this pull request Sep 24, 2025

Roll Flutter from 4a042046a0f1 to 9d203982172f (16 revisions) flutter/packages#10069

Closed

engine-flutter-autoroll mentioned this pull request Sep 24, 2025

Roll Flutter from 4a042046a0f1 to a6d424f7e366 (18 revisions) flutter/packages#10070

Closed

This was referenced Sep 24, 2025

Roll Flutter from 4a042046a0f1 to 439a80965a44 (21 revisions) flutter/packages#10071

Closed

Roll Flutter from 4a042046a0f1 to 321ae32b3eee (23 revisions) flutter/packages#10072

Closed

engine-flutter-autoroll mentioned this pull request Sep 24, 2025

Roll Flutter from 4a042046a0f1 to 440e7c2978dd (24 revisions) flutter/packages#10073

Closed

This was referenced Sep 25, 2025

Roll Flutter from 4a042046a0f1 to 57b7c46d0120 (43 revisions) flutter/packages#10076

Closed

Roll Flutter from 4a042046a0f1 to b1a28bc065b0 (44 revisions) flutter/packages#10077

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size #175450

Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size #175450

Uh oh!

jason-simmons commented Sep 16, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Sep 16, 2025

Uh oh!

jason-simmons Sep 16, 2025

Uh oh!

gemini-code-assist bot Sep 16, 2025

Uh oh!

chinmaygarde Sep 16, 2025 •

edited

Loading

Uh oh!

jason-simmons Sep 16, 2025

Uh oh!

chinmaygarde Sep 16, 2025

Uh oh!

jason-simmons Sep 16, 2025

Uh oh!

chinmaygarde left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size #175450

Do not present textures in FlutterMetalLayer if the drawable size changed and the texture's size does not match the new drawable size #175450

Uh oh!

Conversation

jason-simmons commented Sep 16, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chinmaygarde Sep 16, 2025 •

edited

Loading