Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flame_riverpod forceBuild can call setState() during build #3354

Open
1 task
TonyDowney opened this issue Oct 26, 2024 · 4 comments
Open
1 task

flame_riverpod forceBuild can call setState() during build #3354

TonyDowney opened this issue Oct 26, 2024 · 4 comments
Labels

Comments

@TonyDowney
Copy link

What happened?

I have a FlameGame using riverpod that is moved around the widget tree quite a bit, and reparented. Typically it's fine, but about 5% of the time an error is thrown: setState() or markNeedsBuild() called during build.

Full error log below, but the important part is #3, the forceBuild method, which can be called during an existing widget build, but has no protections from calling setState during a build.

setState() or markNeedsBuild() called during build.
This RiverpodAwareGameWidget<FooGame> widget cannot be marked as needing to build because the framework is already in the process of building widgets. A widget can be marked as needing to be built during the build phase only if one of its ancestors is currently building. This exception is allowed because the framework builds parent widgets before children, which means a dirty descendant will always be built. Otherwise, the framework might not visit this widget during this build phase.
The widget on which setState() or markNeedsBuild() was called was:
  RiverpodAwareGameWidget<FooGame>-[LabeledGlobalKey<RiverpodAwareGameWidgetState<FooGame>>#d5fcf]
The widget which was currently being built when the offending call was made was:
  LayoutBuilder
      #0      Element.markNeedsBuild.<anonymous closure> (package:flutter/src/widgets/framework.dart:5177:9)
      #1      Element.markNeedsBuild (package:flutter/src/widgets/framework.dart:5189:6)
      #2      State.setState (package:flutter/src/widgets/framework.dart:1223:15)
      #3      RiverpodAwareGameWidgetState.forceBuild (package:flame_riverpod/src/widget.dart:64:5)
      #4      RiverpodComponentMixin.rebuildGameWidget (package:flame_riverpod/src/consumer.dart:124:42)
      #5      RiverpodComponentMixin.onRemove (package:flame_riverpod/src/consumer.dart:112:7)
      #6      Component._remove.<anonymous closure> (package:flame/src/components/core/component.dart:984:13)
      #7      Iterable.every (dart:core/iterable.dart:427:16)
      #8      Component.propagateToChildren (package:flame/src/components/core/component.dart:387:10)
      #9      Component._remove (package:flame/src/components/core/component.dart:981:5)
      #10     Component.handleLifecycleEventRemove (package:flame/src/components/core/component.dart:840:7)
      #11     ComponentTreeRoot.processLifecycleEvents (package:flame/src/components/core/component_tree_root.dart:125:19)
      #12     FlameGame.updateTree (package:flame/src/game/flame_game.dart:158:5)
      #13     FlameGame.update (package:flame/src/game/flame_game.dart:152:7)
      #14     GameWidgetState.build.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:flame/src/game/game_widget/game_widget.dart:387:37)
      #15     GameWidgetState._protectedBuild (package:flame/src/game/game_widget/game_widget.dart:228:21)
      #16     GameWidgetState.build.<anonymous closure>.<anonymous closure> (package:flame/src/game/game_widget/game_widget.dart:376:28)
      #17     _LayoutBuilderElement._rebuildWithConstraints.updateChildCallback (package:flutter/src/widgets/layout_builder.dart:191:77)
      #18     BuildOwner.buildScope (package:flutter/src/widgets/framework.dart:3038:19)
      #19     _LayoutBuilderElement._rebuildWithConstraints (package:flutter/src/widgets/layout_builder.dart:231:12)
      #20     RenderObject.invokeLayoutCallback.<anonymous closure> (package:flutter/src/rendering/object.dart:2719:59)
      #21     PipelineOwner._enableMutationsToDirtySubtrees (package:flutter/src/rendering/object.dart:1098:15)
      #22     RenderObject.invokeLayoutCallback (package:flutter/src/rendering/object.dart:2719:14)
      #23     RenderConstrainedLayoutBuilder.rebuildIfNecessary (package:flutter/src/widgets/layout_builder.dart:278:5)
      #24     _RenderLayoutBuilder.performLayout (package:flutter/src/widgets/layout_builder.dart:369:5)
      #25     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #26     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #27     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #28     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #29     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #30     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #31     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #32     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #33     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #34     RenderAspectRatio.performLayout (package:flutter/src/rendering/proxy_box.dart:580:12)
      #35     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #36     RenderPadding.performLayout (package:flutter/src/rendering/shifted_box.dart:234:12)
      #37     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #38     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #39     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #40     RenderPositionedBox.performLayout (package:flutter/src/rendering/shifted_box.dart:451:14)
      #41     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #42     RenderProxyBoxMixin.performLayout (package:flutter/src/rendering/proxy_box.dart:111:21)
      #43     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #44     RenderConstrainedBox.performLayout (package:flutter/src/rendering/proxy_box.dart:291:14)
      #45     RenderObject.layout (package:flutter/src/rendering/object.dart:2608:7)
      #46     MultiChildLayoutDelegate.layoutChild (package:flutter/src/rendering/custom_layout.dart:173:12)
      #47     _ScaffoldLayout.performLayout (package:flutter/src/material/scaffold.dart:1092:7)
      #48     MultiChildLayoutDelegate._callPerformLayout (package:flutter/src/rendering/custom_layout.dart:237:7)
      #49     RenderCustomMultiChildLayoutBox.performLayout (package:flutter/src/rendering/custom_layout.dart:404:14)
      #50     RenderObject._layoutWithoutResize (package:flutter/src/rendering/object.dart:2446:7)
      #51     PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:1052:18)
      #52     PipelineOwner.flushLayout (package:flutter/src/rendering/object.dart:1065:15)
      #53     RendererBinding.drawFrame (package:flutter/src/rendering/binding.dart:602:23)
      #54     WidgetsBinding.drawFrame (package:flutter/src/widgets/binding.dart:1164:13)
      #55     RendererBinding._handlePersistentFrameCallback (package:flutter/src/rendering/binding.dart:468:5)
      #56     SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1397:15)
      #57     SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1318:9)
      #58     SchedulerBinding._handleDrawFrame (package:flutter/src/scheduler/binding.dart:1176:5)
      #59     _invoke (dart:ui/hooks.dart:312:13)
      #60     PlatformDispatcher._drawFrame (dart:ui/platform_dispatcher.dart:419:5)
      #61     _drawFrame (dart:ui/hooks.dart:283:31)

What do you expect?

setState should not be called during build, so the build phase will have to be checked so that setState is either called, or queued.

How can we reproduce this?

Hmm. I was doing it with navigation, but you could try placing a Game in a layoutBuilder that changes the widget tree if the window size is even or odd. That'll trigger dozens of calls to rebuild as you resize a window.

What steps should take to fix this?

No response

Do have an example of where the bug occurs?

No response

Relevant log output

No response

Execute in a terminal and put output into the code block below

No response

Affected platforms

All

Other information

No response

Are you interested in working on a PR for this?

  • I want to work on this
@TonyDowney TonyDowney added the bug label Oct 26, 2024
@spydon
Copy link
Member

spydon commented Oct 26, 2024

@markvideon any clue how this can be avoided?

@TonyDowney
Copy link
Author

addPostFrameCallback is a blunt instrument but it works.

@markvideon
Copy link
Contributor

@markvideon any clue how this can be avoided?

addPostFrameCallback is a blunt instrument but it works.

The implementation of forceBuild currently looks like this:

  void forceBuild() {
    if (_isForceBuilding) {
      _hasQueuedBuild = true;
      return;
    }
    _isForceBuilding = true;
    setState(() {});
  }

and it works in conjunction with this persistent frame callback defined in initState:

    WidgetsBinding.instance.addPersistentFrameCallback((_) {
      _isForceBuilding = false;

      if (_hasQueuedBuild) {
        _hasQueuedBuild = false;
        forceBuild();
      }
    });

I haven't attempted to replicate this but I have some ideas.

  • We probably should check the game widget's context is mounted for scenarios where it could be re-parented
  • We could potentially replace the use of persistent frame callback with individual post frame callbacks when force rebuilds are scheduled
  • We could add checks for SchedulerPhase to determine whether it is safe to call setState within forceBuild, perhaps returning early if we are not in the 'idle' phase. Based on the description of SchedulerPhase.persistentFramecallbacks, we might be in a danger zone using persistent frame callbacks as we are now!

@markvideon
Copy link
Contributor

@TonyDowney I made an attempt to replicate this but so far have been unable to do so. Would it be possible for you to provide a code sample?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants
@spydon @markvideon @TonyDowney and others