Skip to content

[web] Make it safe to call dispose multiple times on a CkSurface#184270

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
jason-simmons:bug_183923_surface_dispose
Mar 30, 2026
Merged

[web] Make it safe to call dispose multiple times on a CkSurface#184270
auto-submit[bot] merged 1 commit into
flutter:masterfrom
jason-simmons:bug_183923_surface_dispose

Conversation

@jason-simmons

@jason-simmons jason-simmons commented Mar 27, 2026

Copy link
Copy Markdown
Member

References to a CkSurface instance may be held by multiple users. Specifically, the PlatformViewEmbedder and the DisplayCanvasFactory can use the same surface, and both classes will dispose that surface during a hot restart.

This PR changes CkSurface.dispose to set _skSurface to null so that future calls to dispose will do nothing.

See #183923

@jason-simmons
[web] Make it safe to call dispose multiple times on a CkSurface
78b6412 
References to a CkSurface instance may be held by multiple users.  Specifically, the PlatformViewEmbedder and the DisplayCanvasFactory can use the same surface, and both classes will dispose that surface during a hot restart.

This PR changes CkSurface.dispose to set _skSurface to null so that future calls to dispose will do nothing.

See flutter#183923
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Mar 27, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Mar 27, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the CkSurface.dispose method to nullify the _skSurface reference and adds a test case to ensure that calling dispose multiple times is safe. A review comment suggests enhancing the new test by adding assertions to verify that skSurface is null after disposal, ensuring the state is correctly updated.

Comment thread engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart
Comment on lines +148 to +151
// Check that it is safe to call dispose multiple times on the same
// surface.
surface.dispose();
surface.dispose();

@gemini-code-assist gemini-code-assist Bot Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This test correctly checks that calling dispose multiple times doesn't cause a crash. However, it could be made more robust by adding assertions to verify the state of the CkSurface after dispose is called. Specifically, you could check that surface.skSurface is non-null before disposal, becomes null after the first call to dispose, and remains null after subsequent calls.

      expect(surface.skSurface, isNotNull);

      surface.dispose();
      expect(surface.skSurface, isNull);

      // The second call should not throw, and the surface should remain disposed.
      surface.dispose();
      expect(surface.skSurface, isNull);

@harryterkelsen harryterkelsen Mar 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know about this suggestion. What we really care about is disposing multiple times doesn't cause a crash, not the implementation details about how that is done.

harryterkelsen
harryterkelsen approved these changes Mar 27, 2026
View reviewed changes

@harryterkelsen harryterkelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@jason-simmons jason-simmons mentioned this pull request Mar 27, 2026
Closed
@harryterkelsen

harryterkelsen commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

This LGTM although IIRC the issue hitting people who run hot restart many times in a row is that the glcontextlost event handler for the disposed Surface will run and cause a crash.

@jason-simmons jason-simmons added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Mar 27, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Mar 30, 2026
Merged via the queue into flutter:master with commit 7587e6e Mar 30, 2026
188 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 30, 2026
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 31, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
d4d145c 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 31, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
b604517 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 31, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
9f2d4a2 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 1, 2026
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
097e262 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 1, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
fd9086f 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 1, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
c841edf 
…rface (flutter/flutter#184270)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 1, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2026
@engine-flutter-autoroll
7587e6e90 [web] Make it safe to call dispose multiple times on a CkSu…
120aa05 
…rface (flutter/flutter#184270)
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 2, 2026
@engine-flutter-autoroll
Roll Flutter from fb03253e32ce to 3d69471c0bf9 (69 revisions) (#11408)
7991bf1 
Roll Flutter from fb03253e32ce to 3d69471c0bf9 (69 revisions)

flutter/flutter@fb03253...3d69471

2026-04-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 1308a3076402 to 043a2bfd56ff (1 revision) (flutter/flutter#184453)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from a657b5446209 to c2363c39c283 (2 revisions) (flutter/flutter#184448)
2026-04-01 104349824+huycozy@users.noreply.github.com Fix layout overflowed in small screen in SensitiveContent's example (flutter/flutter#184179)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from e0b25041a5d5 to a657b5446209 (1 revision) (flutter/flutter#184445)
2026-04-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 75a089eb6bf9 to 1308a3076402 (3 revisions) (flutter/flutter#184444)
2026-04-01 jesswon@google.com [AGP 9] Bumping KGP error minimum to 2.0.0 (flutter/flutter#184385)
2026-04-01 bkonyi@google.com [ Tool ] Migrate `flutter analyze` to use LSP (flutter/flutter#183785)
2026-04-01 30870216+gaaclarke@users.noreply.github.com Adds uber sdf shader gradients with blend (flutter/flutter#184090)
2026-04-01 sys.int64@gmail.com Add bottom safe area padding to licenses package license page (flutter/flutter#182425)
2026-04-01 ahmedsameha1@gmail.com Handle#6537 third grouped tests (flutter/flutter#183059)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from f37239a7a689 to e0b25041a5d5 (9 revisions) (flutter/flutter#184436)
2026-03-31 jason-simmons@users.noreply.github.com [Impeller] Do not log an error when wrapping an empty texture as a TextureGLES (flutter/flutter#184377)
2026-03-31 jason-simmons@users.noreply.github.com Remove the default_git_folder GN argument (flutter/flutter#184152)
2026-03-31 jason-simmons@users.noreply.github.com Remove the cupertino_icons dependency from the spell_check integration test (flutter/flutter#184398)
2026-03-31 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from pM94cWC9cSgao0CG0... to fV-JIWUt4FQGeDtEe... (flutter/flutter#184383)
2026-03-31 engine-flutter-autoroll@skia.org Roll Dart SDK from eaeccf98848d to 75a089eb6bf9 (1 revision) (flutter/flutter#184379)
2026-03-31 mdebbar@google.com [web] Fix autofill in iOS 26 Safari (flutter/flutter#182024)
2026-03-31 engine-flutter-autoroll@skia.org Roll Fuchsia GN SDK from SEfYx3xgueX3aFAY3... to JLBh4Z9PKsjIJcqDU... (flutter/flutter#184368)
2026-03-31 loic.peron@inetum.com [Windows] Restore and enable IAccessibleEx implementation (flutter/flutter#175406)
2026-03-31 30870216+gaaclarke@users.noreply.github.com Revert "Even more awaits (#184042)" (flutter/flutter#184429)
2026-03-31 engine-flutter-autoroll@skia.org Roll Skia from dfd8f8002800 to f37239a7a689 (2 revisions) (flutter/flutter#184374)
2026-03-31 jacksongardner@google.com Remove workaround for fake impeller images in iOS simulator. (flutter/flutter#184264)
2026-03-31 victorsanniay@gmail.com Even more awaits (flutter/flutter#184042)
2026-03-31 engine-flutter-autoroll@skia.org Roll Packages from 582f0e7 to b04f3e5 (6 revisions) (flutter/flutter#184393)
2026-03-30 30870216+gaaclarke@users.noreply.github.com Fixes a flake in reload shaders tests (flutter/flutter#184268)
2026-03-30 jason-simmons@users.noreply.github.com Remove an obsolete script for setting up remote GDB sessions on Android devices (flutter/flutter#184357)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from 8dcde79fef2a to dfd8f8002800 (10 revisions) (flutter/flutter#184363)
2026-03-30 engine-flutter-autoroll@skia.org Roll Dart SDK from 0aaccc3c8004 to eaeccf98848d (2 revisions) (flutter/flutter#184362)
2026-03-30 bkonyi@google.com [ Tool ] Remove `flutter running-apps` command (flutter/flutter#183742)
2026-03-30 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#184045)
2026-03-30 jmccandless@google.com Rick roll triagers on/near April 1st (flutter/flutter#184355)
2026-03-30 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 5.5.0 to 6.0.0 in the all-github-actions group (flutter/flutter#184364)
2026-03-30 1961493+harryterkelsen@users.noreply.github.com fix(web): call ui.Picture.onDispose for the original picture only (flutter/flutter#184348)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from e001e6901e3b to 8dcde79fef2a (7 revisions) (flutter/flutter#184356)
2026-03-30 jason-simmons@users.noreply.github.com [web] Make it safe to call dispose multiple times on a CkSurface (flutter/flutter#184270)
2026-03-30 jason-simmons@users.noreply.github.com Roll HarfBuzz to 13.2.1 (flutter/flutter#184210)
2026-03-30 srawlins@google.com web_ui: Remove unused parameters in a few places (flutter/flutter#183156)
2026-03-30 saurabhmirajkar000@gmail.com Update TabBar documentation to clarify indicatorWeight behavior (flutter/flutter#184104)
2026-03-30 36861262+QuncCccccc@users.noreply.github.com Add title evaluation (flutter/flutter#184084)
2026-03-30 47866232+chunhtai@users.noreply.github.com fixes crash when invisible semantics nodes dropped from semantics tree (flutter/flutter#184226)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from cdaae3e3fdef to e001e6901e3b (4 revisions) (flutter/flutter#184345)
2026-03-30 engine-flutter-autoroll@skia.org Roll Packages from 7ae082a to 582f0e7 (8 revisions) (flutter/flutter#184341)
2026-03-30 matej.knopp@gmail.com Add alwaysSizeToContent argument to Overlay. (flutter/flutter#182009)
2026-03-30 engine-flutter-autoroll@skia.org Roll Dart SDK from 598088a8a67f to 0aaccc3c8004 (1 revision) (flutter/flutter#184331)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from 6d7ade938643 to cdaae3e3fdef (2 revisions) (flutter/flutter#184329)
2026-03-30 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from EnoD6zNQebz4EYuLk... to pM94cWC9cSgao0CG0... (flutter/flutter#184323)
...
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Apr 14, 2026
@jason-simmons @ahmedsameha1
[web] Make it safe to call dispose multiple times on a CkSurface (flu…
70cf61b 
…tter#184270)

References to a CkSurface instance may be held by multiple users.
Specifically, the PlatformViewEmbedder and the DisplayCanvasFactory can
use the same surface, and both classes will dispose that surface during
a hot restart.

This PR changes CkSurface.dispose to set _skSurface to null so that
future calls to dispose will do nothing.

See flutter#183923
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
@engine-flutter-autoroll
Roll Flutter from fb03253e32ce to 3d69471c0bf9 (69 revisions) (flutte…
ce28649 
…r#11408)

Roll Flutter from fb03253e32ce to 3d69471c0bf9 (69 revisions)

flutter/flutter@fb03253...3d69471

2026-04-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 1308a3076402 to 043a2bfd56ff (1 revision) (flutter/flutter#184453)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from a657b5446209 to c2363c39c283 (2 revisions) (flutter/flutter#184448)
2026-04-01 104349824+huycozy@users.noreply.github.com Fix layout overflowed in small screen in SensitiveContent's example (flutter/flutter#184179)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from e0b25041a5d5 to a657b5446209 (1 revision) (flutter/flutter#184445)
2026-04-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 75a089eb6bf9 to 1308a3076402 (3 revisions) (flutter/flutter#184444)
2026-04-01 jesswon@google.com [AGP 9] Bumping KGP error minimum to 2.0.0 (flutter/flutter#184385)
2026-04-01 bkonyi@google.com [ Tool ] Migrate `flutter analyze` to use LSP (flutter/flutter#183785)
2026-04-01 30870216+gaaclarke@users.noreply.github.com Adds uber sdf shader gradients with blend (flutter/flutter#184090)
2026-04-01 sys.int64@gmail.com Add bottom safe area padding to licenses package license page (flutter/flutter#182425)
2026-04-01 ahmedsameha1@gmail.com Handle#6537 third grouped tests (flutter/flutter#183059)
2026-04-01 engine-flutter-autoroll@skia.org Roll Skia from f37239a7a689 to e0b25041a5d5 (9 revisions) (flutter/flutter#184436)
2026-03-31 jason-simmons@users.noreply.github.com [Impeller] Do not log an error when wrapping an empty texture as a TextureGLES (flutter/flutter#184377)
2026-03-31 jason-simmons@users.noreply.github.com Remove the default_git_folder GN argument (flutter/flutter#184152)
2026-03-31 jason-simmons@users.noreply.github.com Remove the cupertino_icons dependency from the spell_check integration test (flutter/flutter#184398)
2026-03-31 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from pM94cWC9cSgao0CG0... to fV-JIWUt4FQGeDtEe... (flutter/flutter#184383)
2026-03-31 engine-flutter-autoroll@skia.org Roll Dart SDK from eaeccf98848d to 75a089eb6bf9 (1 revision) (flutter/flutter#184379)
2026-03-31 mdebbar@google.com [web] Fix autofill in iOS 26 Safari (flutter/flutter#182024)
2026-03-31 engine-flutter-autoroll@skia.org Roll Fuchsia GN SDK from SEfYx3xgueX3aFAY3... to JLBh4Z9PKsjIJcqDU... (flutter/flutter#184368)
2026-03-31 loic.peron@inetum.com [Windows] Restore and enable IAccessibleEx implementation (flutter/flutter#175406)
2026-03-31 30870216+gaaclarke@users.noreply.github.com Revert "Even more awaits (#184042)" (flutter/flutter#184429)
2026-03-31 engine-flutter-autoroll@skia.org Roll Skia from dfd8f8002800 to f37239a7a689 (2 revisions) (flutter/flutter#184374)
2026-03-31 jacksongardner@google.com Remove workaround for fake impeller images in iOS simulator. (flutter/flutter#184264)
2026-03-31 victorsanniay@gmail.com Even more awaits (flutter/flutter#184042)
2026-03-31 engine-flutter-autoroll@skia.org Roll Packages from 582f0e7 to b04f3e5 (6 revisions) (flutter/flutter#184393)
2026-03-30 30870216+gaaclarke@users.noreply.github.com Fixes a flake in reload shaders tests (flutter/flutter#184268)
2026-03-30 jason-simmons@users.noreply.github.com Remove an obsolete script for setting up remote GDB sessions on Android devices (flutter/flutter#184357)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from 8dcde79fef2a to dfd8f8002800 (10 revisions) (flutter/flutter#184363)
2026-03-30 engine-flutter-autoroll@skia.org Roll Dart SDK from 0aaccc3c8004 to eaeccf98848d (2 revisions) (flutter/flutter#184362)
2026-03-30 bkonyi@google.com [ Tool ] Remove `flutter running-apps` command (flutter/flutter#183742)
2026-03-30 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#184045)
2026-03-30 jmccandless@google.com Rick roll triagers on/near April 1st (flutter/flutter#184355)
2026-03-30 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 5.5.0 to 6.0.0 in the all-github-actions group (flutter/flutter#184364)
2026-03-30 1961493+harryterkelsen@users.noreply.github.com fix(web): call ui.Picture.onDispose for the original picture only (flutter/flutter#184348)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from e001e6901e3b to 8dcde79fef2a (7 revisions) (flutter/flutter#184356)
2026-03-30 jason-simmons@users.noreply.github.com [web] Make it safe to call dispose multiple times on a CkSurface (flutter/flutter#184270)
2026-03-30 jason-simmons@users.noreply.github.com Roll HarfBuzz to 13.2.1 (flutter/flutter#184210)
2026-03-30 srawlins@google.com web_ui: Remove unused parameters in a few places (flutter/flutter#183156)
2026-03-30 saurabhmirajkar000@gmail.com Update TabBar documentation to clarify indicatorWeight behavior (flutter/flutter#184104)
2026-03-30 36861262+QuncCccccc@users.noreply.github.com Add title evaluation (flutter/flutter#184084)
2026-03-30 47866232+chunhtai@users.noreply.github.com fixes crash when invisible semantics nodes dropped from semantics tree (flutter/flutter#184226)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from cdaae3e3fdef to e001e6901e3b (4 revisions) (flutter/flutter#184345)
2026-03-30 engine-flutter-autoroll@skia.org Roll Packages from 7ae082a to 582f0e7 (8 revisions) (flutter/flutter#184341)
2026-03-30 matej.knopp@gmail.com Add alwaysSizeToContent argument to Overlay. (flutter/flutter#182009)
2026-03-30 engine-flutter-autoroll@skia.org Roll Dart SDK from 598088a8a67f to 0aaccc3c8004 (1 revision) (flutter/flutter#184331)
2026-03-30 engine-flutter-autoroll@skia.org Roll Skia from 6d7ade938643 to cdaae3e3fdef (2 revisions) (flutter/flutter#184329)
2026-03-30 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from EnoD6zNQebz4EYuLk... to pM94cWC9cSgao0CG0... (flutter/flutter#184323)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harryterkelsen harryterkelsen harryterkelsen approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jason-simmons @harryterkelsen