Fix debugNeedsPaint/Layout/CompositedLayerUpdate crashing in release mode#184627
Conversation
…mode Replace the legacy `late bool result` + `assert()` pattern with an explicit `kReleaseMode` guard, matching the existing `debugNeedsSemanticsUpdate` getter in the same class. The old pattern caused LateInitializationError in release builds because Dart strips assert() blocks. Fixes flutter#183696
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request modifies RenderObject to return false for debugNeedsLayout, debugNeedsPaint, and debugNeedsCompositedLayerUpdate in release builds, replacing the previous behavior that would throw when asserts were disabled. The review feedback recommends using !kDebugMode instead of kReleaseMode to ensure consistent behavior in both profile and release modes and suggests updating the documentation to reflect that these properties return false in all non-debug builds.
| /// out and painted. Always returns false in release builds. | ||
| /// | ||
| /// It is intended to be used by tests and asserts. | ||
| bool get debugNeedsLayout { | ||
| late bool result; | ||
| assert(() { | ||
| result = _needsLayout; | ||
| return true; | ||
| }()); | ||
| return result; | ||
| if (kReleaseMode) { | ||
| return false; | ||
| } | ||
| return _needsLayout; |
There was a problem hiding this comment.
Using kReleaseMode only handles the release build case. However, since assert blocks are also disabled in profile mode, the original implementation would have crashed there as well. To ensure consistent behavior across all non-debug builds (Profile and Release) and to follow the convention for debug* properties, consider using !kDebugMode instead. This ensures the property returns false in both profile and release modes, and the documentation should be updated to reflect this.
/// out and painted. Always returns false in non-debug builds.
///
/// It is intended to be used by tests and asserts.
bool get debugNeedsLayout {
if (!kDebugMode) {
return false;
}
return _needsLayout;References
- Code should follow the guidance and principles described in the Flutter contribution guide, which includes doing things right for all build modes. (link)
| /// to condition their runtime behavior on whether they are dirty or not, | ||
| /// since they should only be marked dirty immediately prior to being laid | ||
| /// out and painted. (In release builds, this throws.) | ||
| /// out and painted. Always returns false in release builds. |
| if (kReleaseMode) { | ||
| return false; | ||
| } | ||
| return _needsPaint; |
There was a problem hiding this comment.
| /// out and painted. Always returns false in release builds. | ||
| /// | ||
| /// It is intended to be used by tests and asserts. | ||
| bool get debugNeedsCompositedLayerUpdate { | ||
| late bool result; | ||
| assert(() { | ||
| result = _needsCompositedLayerUpdate; | ||
| return true; | ||
| }()); | ||
| return result; | ||
| if (kReleaseMode) { | ||
| return false; | ||
| } | ||
| return _needsCompositedLayerUpdate; |
There was a problem hiding this comment.
Similar to the other debug properties, using !kDebugMode is preferred over kReleaseMode to provide consistent behavior across all non-debug builds (Profile and Release) and to match the updated documentation.
/// out and painted. Always returns false in non-debug builds.
///
/// It is intended to be used by tests and asserts.
bool get debugNeedsCompositedLayerUpdate {
if (!kDebugMode) {
return false;
}
return _needsCompositedLayerUpdate;- Use !kDebugMode to cover both profile and release modes, since asserts are disabled in both - Update doc comments to say "non-debug builds" instead of "release builds"
|
These are not meant to be called in release mode. I wonder if the fix is in the other direction, fixing the places where this pattern is not honored. I need to look into the other cases where tis has been allowed. |
|
Check this out, I tried a similar PR, landed an a linter instead #183697 |
|
Touching base with @chunhtai about this, I see the issue references how debugNeedsSemanticsUpdate works, which was introduced in https://github.com/flutter/flutter/pull/151688/files |
Piinks
left a comment
There was a problem hiding this comment.
Caught up with @chunhtai on this, we both agree that debug method is ok to be called outside of debug mode, but they won't return meaningful result. I think this LGTM as it aligns with debugNeedsSemanticsUpdate, and we are looking into comprehensive lint coverage in #185091
Thanks for you patience while I double checked this.
|
autosubmit label was removed for flutter/flutter/184627, because - The status or check suite Windows framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…11595) Manual roll Flutter from 31001886cbe8 to 61fca76dd523 (53 revisions) Manual roll requested by tarrinneal@google.com flutter/flutter@3100188...61fca76 2026-04-27 30870216+gaaclarke@users.noreply.github.com Adds integration test for the FLTEnableSDFs flag for iOS (flutter/flutter#185637) 2026-04-27 97480502+b-luk@users.noreply.github.com Fix sdfs being enabled for MacOS regardless of FLTEnableSDFs value (flutter/flutter#185565) 2026-04-27 97480502+b-luk@users.noreply.github.com Don't use UberSDF for paint with incompatible blend modes (flutter/flutter#184889) 2026-04-27 engine-flutter-autoroll@skia.org Roll Dart SDK from de495e3de9a0 to 941ca325cfc9 (2 revisions) (flutter/flutter#185653) 2026-04-27 jhy03261997@gmail.com [a11y] Add CONTENT_CHANGE_TYPE_EXPANDED support on android. (flutter/flutter#185305) 2026-04-27 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#185641) 2026-04-27 1063596+reidbaker@users.noreply.github.com Modify analyze.dart to have flags to run only or exclude each verification step. (flutter/flutter#185618) 2026-04-27 meylis@divine.video Fix debugNeedsPaint/Layout/CompositedLayerUpdate crashing in release mode (flutter/flutter#184627) 2026-04-27 kallentu@google.com Enable `var_with_no_type_annotation` lint. (flutter/flutter#185215) 2026-04-27 53523825+JhonaCodes@users.noreply.github.com Fix SelectionArea handles overlapping context menu on Android (flutter/flutter#182663) 2026-04-27 30870216+gaaclarke@users.noreply.github.com Adds debugging information to compiled metal shaders (flutter/flutter#185629) 2026-04-27 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#185638) 2026-04-27 84978733+alejandro-all-win-software@users.noreply.github.com Use null-aware elements in dev/devicelab/lib/framework/browser.dart (flutter/flutter#184778) 2026-04-27 154381524+flutteractionsbot@users.noreply.github.com Sync CHANGELOG.md from stable (flutter/flutter#185633) 2026-04-27 31591868+zawhtetnaing10@users.noreply.github.com Added useOriginalColors flag which allows ImageIcon to bypass IconTheme colorization and use the original colors (flutter/flutter#180491) 2026-04-27 engine-flutter-autoroll@skia.org Roll Packages from 8400f71 to 23280da (2 revisions) (flutter/flutter#185619) 2026-04-27 engine-flutter-autoroll@skia.org Roll Skia from f1238e0f1022 to ce82d32b3e03 (1 revision) (flutter/flutter#185616) 2026-04-27 15619084+vashworth@users.noreply.github.com [SwiftPM] Enable package resolution on xcodebuild commands (flutter/flutter#185208) 2026-04-27 chris@bracken.jp [iOS] Refactor keyboard inset logic into FlutterKeyboardInsetManager (flutter/flutter#185535) 2026-04-27 matej.knopp@gmail.com [Win32] FlutterDesktopEngineGetGraphicsAdapter should use out parameter (flutter/flutter#185590) 2026-04-27 engine-flutter-autoroll@skia.org Roll Skia from d77e3356d526 to f1238e0f1022 (4 revisions) (flutter/flutter#185604) 2026-04-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from flsn8jC4LkTn6WECf... to i6d0NoDueUiXpePfX... (flutter/flutter#185601) 2026-04-26 engine-flutter-autoroll@skia.org Roll Dart SDK from a108dfe2d227 to de495e3de9a0 (1 revision) (flutter/flutter#185599) 2026-04-26 engine-flutter-autoroll@skia.org Roll Skia from ce9aa2231292 to d77e3356d526 (1 revision) (flutter/flutter#185596) 2026-04-26 engine-flutter-autoroll@skia.org Roll Skia from 622fff4c24d2 to ce9aa2231292 (1 revision) (flutter/flutter#185588) 2026-04-25 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 9fPnyEo9PaNdXtasl... to flsn8jC4LkTn6WECf... (flutter/flutter#185585) 2026-04-25 engine-flutter-autoroll@skia.org Roll Dart SDK from 605a8faf0dda to a108dfe2d227 (1 revision) (flutter/flutter#185584) 2026-04-25 engine-flutter-autoroll@skia.org Roll Dart SDK from 6b8c39765f17 to 605a8faf0dda (1 revision) (flutter/flutter#185578) 2026-04-25 engine-flutter-autoroll@skia.org Roll Skia from 185f6b57d64f to 622fff4c24d2 (1 revision) (flutter/flutter#185573) 2026-04-25 evanwall@buffalo.edu Implement square-like round superellipses with circular corners in the SDF uber shader (flutter/flutter#185370) 2026-04-25 victorsanniay@gmail.com Add @awaitNotRequired annotation to flutter sdk (flutter/flutter#181513) 2026-04-25 engine-flutter-autoroll@skia.org Roll Dart SDK from 01228cb7af42 to 6b8c39765f17 (2 revisions) (flutter/flutter#185569) 2026-04-25 flar@google.com Adapt the DisplayList benchmarks into a primitive rendering benchmark suite (flutter/flutter#185270) 2026-04-25 okorohelijah@google.com Enable SPM for GoogleMobileAds (flutter/flutter#185548) 2026-04-25 engine-flutter-autoroll@skia.org Roll Skia from 3f467a581942 to 185f6b57d64f (1 revision) (flutter/flutter#185564) 2026-04-25 victorsanniay@gmail.com Fix Table crash when a cell child paints below the row bottom (flutter/flutter#185323) 2026-04-24 ahmedsameha1@gmail.com Make sure that an Image doesn't crash in 0x0 environment (flutter/flutter#181154) 2026-04-24 engine-flutter-autoroll@skia.org Roll Skia from 300d432048b0 to 3f467a581942 (3 revisions) (flutter/flutter#185558) 2026-04-24 engine-flutter-autoroll@skia.org Roll Dart SDK from c26627715892 to 01228cb7af42 (4 revisions) (flutter/flutter#185559) 2026-04-24 jacksongardner@google.com Reland "[web] Fix LateInitializationError in CkSurface and SkwasmSurface (#185116)" (flutter/flutter#185553) 2026-04-24 41930132+hellohuanlin@users.noreply.github.com [github]fix git ls-file glob pattern in labeler.yml instruction (flutter/flutter#185495) 2026-04-24 41930132+hellohuanlin@users.noreply.github.com [ios]update ios-reviewers tags to include more files (flutter/flutter#185490) 2026-04-24 tomac@google.com Add initial support for Cross-Origin Storage (flutter/flutter#184149) 2026-04-24 30870216+gaaclarke@users.noreply.github.com tool: Skip cached engine artifacts with local engine (flutter/flutter#185546) ...
Summary
debugNeedsPaint,debugNeedsLayout, anddebugNeedsCompositedLayerUpdateuse alatevariable inside anassert()closure, which means the variable is never initialized in release mode — causing aLateInitializationErrorcrash when accessedlate+assertpattern with an explicitkReleaseModecheck that returnsfalsein release builds, matching the documented intentChanges
packages/flutter/lib/src/rendering/object.dart: Updated all 3 debug getters to usekReleaseModeguard instead oflate+assertpatternFixes #183696