Fix vsync race condition on providerscope unmount (#3072)

fixes #2022
rrousselGit · Oct 31, 2023 · dd3c7df · dd3c7df
1 parent 66e7777
commit dd3c7df
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 10 deletions.
diff --git a/packages/flutter_riverpod/CHANGELOG.md b/packages/flutter_riverpod/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - Exceptions in asynchronous providers are now correctly received
   by `ProviderObserver.providerDidFail`.
+- Fix exception when a `ProviderScope` is rebuilt with a different `key`.
 
 ## 2.4.5 - 2023-10-28
 

diff --git a/packages/flutter_riverpod/lib/src/framework.dart b/packages/flutter_riverpod/lib/src/framework.dart
@@ -305,7 +305,7 @@ class _UncontrolledProviderScopeElement extends InheritedElement {
       debugCanModifyProviders ??= _debugCanModifyProviders;
     }
 
-    vsyncOverride ??= _flutterVsync;
+    flutterVsyncs.add(_flutterVsync);
     super.mount(parent, newSlot);
   }
 
@@ -380,9 +380,7 @@ To fix this problem, you have one of two solutions:
       debugCanModifyProviders = null;
     }
 
-    if (vsyncOverride == _flutterVsync) {
-      vsyncOverride = null;
-    }
+    flutterVsyncs.remove(_flutterVsync);
 
     super.unmount();
   }

diff --git a/packages/flutter_riverpod/test/framework_test.dart b/packages/flutter_riverpod/test/framework_test.dart
@@ -10,6 +10,43 @@ import 'package:mockito/mockito.dart';
 import 'utils.dart';
 
 void main() {
+  testWidgets('Supports recreating ProviderScope', (tester) async {
+    final provider = Provider<String>((ref) => 'foo');
+
+    await tester.pumpWidget(
+      ProviderScope(
+        child: Consumer(
+          builder: (context, ref, _) {
+            ref.watch(provider);
+            return Consumer(
+              builder: (context, ref, _) {
+                ref.watch(provider);
+                return Container();
+              },
+            );
+          },
+        ),
+      ),
+    );
+
+    await tester.pumpWidget(
+      ProviderScope(
+        key: UniqueKey(),
+        child: Consumer(
+          builder: (context, ref, _) {
+            ref.watch(provider);
+            return Consumer(
+              builder: (context, ref, _) {
+                ref.watch(provider);
+                return Container();
+              },
+            );
+          },
+        ),
+      ),
+    );
+  });
+
   testWidgets('ref.invalidate can invalidate a family', (tester) async {
     final listener = Listener<String>();
     final listener2 = Listener<String>();
@@ -248,7 +285,27 @@ void main() {
       (tester) async {
     final container = createContainer();
 
-    expect(vsyncOverride, null);
+    expect(flutterVsyncs, isEmpty);
+
+    await tester.pumpWidget(
+      UncontrolledProviderScope(
+        container: container,
+        child: Container(),
+      ),
+    );
+
+    expect(flutterVsyncs, hasLength(1));
+
+    await tester.pumpWidget(
+      UncontrolledProviderScope(
+        container: container,
+        child: ProviderScope(
+          child: Container(),
+        ),
+      ),
+    );
+
+    expect(flutterVsyncs, hasLength(2));
 
     await tester.pumpWidget(
       UncontrolledProviderScope(
@@ -257,11 +314,51 @@ void main() {
       ),
     );
 
-    expect(vsyncOverride, isNotNull);
+    expect(flutterVsyncs, hasLength(1));
 
     await tester.pumpWidget(Container());
 
-    expect(vsyncOverride, null);
+    expect(flutterVsyncs, isEmpty);
+  });
+
+  testWidgets('When there are multiple vsyncs, rebuild providers only once',
+      (tester) async {
+    var buildCount = 0;
+    final dep = StateProvider((ref) => 0);
+    final provider = Provider((ref) {
+      buildCount++;
+      return ref.watch(dep);
+    });
+
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: ProviderScope(
+          child: ProviderScope(
+            child: Consumer(
+              builder: (context, ref, _) {
+                return Text('Hello ${ref.watch(provider)}');
+              },
+            ),
+          ),
+        ),
+      ),
+    );
+
+    expect(buildCount, 1);
+    expect(find.text('Hello 0'), findsOneWidget);
+    expect(find.text('Hello 1'), findsNothing);
+
+    final consumerElement = tester.element(find.byType(Consumer));
+    final container = ProviderScope.containerOf(consumerElement);
+
+    container.read(dep.notifier).state++;
+
+    await tester.pump();
+
+    expect(buildCount, 2);
+    expect(find.text('Hello 1'), findsOneWidget);
+    expect(find.text('Hello 0'), findsNothing);
   });
 
   testWidgets(

diff --git a/packages/riverpod/CHANGELOG.md b/packages/riverpod/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - Exceptions in asynchronous providers are now correctly received
   by `ProviderObserver.providerDidFail`.
+- Fix exception when a `ProviderScope` is rebuilt with a different `key`.
 
 ## 2.4.5 - 2023-10-28
 

diff --git a/packages/riverpod/lib/riverpod.dart b/packages/riverpod/lib/riverpod.dart
@@ -26,7 +26,8 @@ export 'src/framework.dart'
     hide
         debugCanModifyProviders,
         vsync,
-        vsyncOverride,
+        flutterVsyncs,
+        Vsync,
         ValueProviderElement,
         ValueProvider,
         FamilyCreate,

diff --git a/packages/riverpod/lib/src/framework/scheduler.dart b/packages/riverpod/lib/src/framework/scheduler.dart
@@ -1,9 +1,14 @@
 part of '../framework.dart';
 
+/// A listener used to determine when providers should rebuild.
+/// This is used to synchronize provider rebuilds when widget rebuilds.
+@internal
+typedef Vsync = void Function(void Function());
+
 /// A way to override [vsync], used by Flutter to synchronize a container
 /// with the widget tree.
 @internal
-void Function(void Function() task)? vsyncOverride;
+final flutterVsyncs = <Vsync>{};
 
 void _defaultVsync(void Function() task) {
   Future(task);
@@ -14,7 +19,25 @@ void _defaultVsync(void Function() task) {
 /// Defaults to refreshing providers at the end of the next event-loop.
 @internal
 void Function(void Function()) get vsync {
-  return vsyncOverride ?? _defaultVsync;
+  if (flutterVsyncs.isNotEmpty) {
+    // Notify all InheritedWidgets of a possible rebuild.
+    // At the same time, we only execute the task once, in whichever
+    // InheritedWidget that rebuilds first.
+    return (task) {
+      var invoked = false;
+      void invoke() {
+        if (invoked) return;
+        invoked = true;
+        task();
+      }
+
+      for (final flutterVsync in flutterVsyncs) {
+        flutterVsync(invoke);
+      }
+    };
+  }
+
+  return _defaultVsync;
 }
 
 /// The object that handles when providers are refreshed and disposed.