Fix selectAsync sometimes never resolving (#3402)

fixes #2553

packages/flutter_riverpod/lib/src/consumer.dart

@@ -642,8 +642,14 @@ class ConsumerStatefulElement extends StatefulElement implements WidgetRef {
     _assertNotDisposed();
     final listeners = _manualListeners ??= [];
 
+    // Reading the container using "listen:false" to guarantee that this can
+    // be used inside initState.
+    final container = ProviderScope.containerOf(this, listen: false);
+
     final sub = _ListenManual(
-      ProviderScope.containerOf(this, listen: false).listen(
+      // TODO somehow pass "this" instead for the devtool's sake
+      container,
+      container.listen(
         provider,
         listener,
         onError: onError,
@@ -660,16 +666,19 @@ class ConsumerStatefulElement extends StatefulElement implements WidgetRef {
   BuildContext get context => this;
 }
 
-class _ListenManual<T> implements ProviderSubscription<T> {
-  _ListenManual(this._subscription, this._element);
+class _ListenManual<T> extends ProviderSubscription<T> {
+  _ListenManual(super.source, this._subscription, this._element);
 
   final ProviderSubscription<T> _subscription;
   final ConsumerStatefulElement _element;
 
   @override
   void close() {
-    _subscription.close();
-    _element._manualListeners?.remove(this);
+    if (!closed) {
+      _subscription.close();
+      _element._manualListeners?.remove(this);
+    }
+    super.close();
   }
 
   @override

packages/riverpod/CHANGELOG.md

@@ -4,6 +4,8 @@
 - Fix out of date `pub.dev` description
 - `ref.invalidate` now correctly clear all resources associated
   with the provider if the provider is no-longer used.
+- Fix `selectAsync` sometimes never resolving.
+- Fix `ProviderSubscription.read` returned by `ref.listen(provider.future)` not throwing if used after the subscription has been closed.
 
 ## 2.5.0 - 2024-02-03
 

packages/riverpod/lib/src/async_notifier/base.dart

@@ -466,9 +466,8 @@ mixin FutureHandlerProviderElementMixin<T>
 
   @override
   void visitChildren({
-    required void Function(ProviderElementBase<Object?> element) elementVisitor,
-    required void Function(ProxyElementValueNotifier<Object?> element)
-        notifierVisitor,
+    required void Function(ProviderElementBase element) elementVisitor,
+    required void Function(ProxyElementValueNotifier element) notifierVisitor,
   }) {
     super.visitChildren(
       elementVisitor: elementVisitor,
@@ -491,9 +490,8 @@ abstract class AsyncNotifierProviderElementBase<
 
   @override
   void visitChildren({
-    required void Function(ProviderElementBase<Object?> element) elementVisitor,
-    required void Function(ProxyElementValueNotifier<Object?> element)
-        notifierVisitor,
+    required void Function(ProviderElementBase element) elementVisitor,
+    required void Function(ProxyElementValueNotifier element) notifierVisitor,
   }) {
     super.visitChildren(
       elementVisitor: elementVisitor,

packages/riverpod/lib/src/common/env.dart

@@ -0,0 +1,82 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: do_not_use_environment, comment_references
+
+/// A constant that is true if the application was compiled in release mode.
+///
+/// More specifically, this is a constant that is true if the application was
+/// compiled in Dart with the '-Ddart.vm.product=true' flag.
+///
+/// Since this is a const value, it can be used to indicate to the compiler that
+/// a particular block of code will not be executed in release mode, and hence
+/// can be removed.
+///
+/// Generally it is better to use [kDebugMode] or `assert` to gate code, since
+/// using [kReleaseMode] will introduce differences between release and profile
+/// builds, which makes performance testing less representative.
+///
+/// See also:
+///
+///  * [kDebugMode], which is true in debug builds.
+///  * [kProfileMode], which is true in profile builds.
+const bool kReleaseMode = bool.fromEnvironment('dart.vm.product');
+
+/// A constant that is true if the application was compiled in profile mode.
+///
+/// More specifically, this is a constant that is true if the application was
+/// compiled in Dart with the '-Ddart.vm.profile=true' flag.
+///
+/// Since this is a const value, it can be used to indicate to the compiler that
+/// a particular block of code will not be executed in profile mode, an hence
+/// can be removed.
+///
+/// See also:
+///
+///  * [kDebugMode], which is true in debug builds.
+///  * [kReleaseMode], which is true in release builds.
+const bool kProfileMode = bool.fromEnvironment('dart.vm.profile');
+
+/// A constant that is true if the application was compiled in debug mode.
+///
+/// More specifically, this is a constant that is true if the application was
+/// not compiled with '-Ddart.vm.product=true' and '-Ddart.vm.profile=true'.
+///
+/// Since this is a const value, it can be used to indicate to the compiler that
+/// a particular block of code will not be executed in debug mode, and hence
+/// can be removed.
+///
+/// An alternative strategy is to use asserts, as in:
+///
+/// ```dart
+/// assert(() {
+///   // ...debug-only code here...
+///   return true;
+/// }());
+/// ```
+///
+/// See also:
+///
+///  * [kReleaseMode], which is true in release builds.
+///  * [kProfileMode], which is true in profile builds.
+const bool kDebugMode = !kReleaseMode && !kProfileMode;
+
+/// The epsilon of tolerable double precision error.
+///
+/// This is used in various places in the framework to allow for floating point
+/// precision loss in calculations. Differences below this threshold are safe to
+/// disregard.
+const double precisionErrorTolerance = 1e-10;
+
+/// A constant that is true if the application was compiled to run on the web.
+///
+/// See also:
+///
+/// * [defaultTargetPlatform], which is used by themes to find out which
+///   platform the application is running on (or, in the case of a web app,
+///   which platform the application's browser is running in). Can be overridden
+///   in tests with [debugDefaultTargetPlatformOverride].
+/// * [dart:io.Platform], a way to find out the browser's platform that is not
+///   overridable in tests.
+const bool kIsWeb = bool.fromEnvironment('dart.library.js_util');

packages/riverpod/lib/src/framework.dart

@@ -3,8 +3,10 @@ library framework;
 import 'dart:async';
 import 'dart:collection';
 
+import 'package:collection/collection.dart';
 import 'package:meta/meta.dart';
 import 'package:state_notifier/state_notifier.dart';
+import 'common/env.dart';
 import 'internals.dart';
 
 part 'framework/always_alive.dart';

packages/riverpod/lib/src/framework/async_selector.dart

@@ -234,8 +234,18 @@ class _AsyncSelector<Input, Output> with ProviderListenable<Future<Output>> {
     }
 
     return _SelectorSubscription(
+      node,
       sub,
       () => selectedFuture!,
+      onClose: () {
+        final completer = selectedCompleter;
+        if (completer != null && !completer.isCompleted) {
+          read(node).then(
+            completer.complete,
+            onError: completer.completeError,
+          );
+        }
+      },
     );
   }
 

packages/riverpod/lib/src/framework/container.dart

@@ -30,11 +30,11 @@ class _StateReader {
   /// at the creation of the [ProviderContainer]
   final bool isDynamicallyCreated;
 
-  ProviderElementBase<Object?>? _element;
+  ProviderElementBase? _element;
 
-  ProviderElementBase<Object?> getElement() => _element ??= _create();
+  ProviderElementBase getElement() => _element ??= _create();
 
-  ProviderElementBase<Object?> _create() {
+  ProviderElementBase _create() {
     if (origin == _circularDependencyLock) {
       throw CircularDependencyError._();
     }
@@ -268,23 +268,6 @@ class ProviderContainer implements Node {
     );
   }
 
-  @override
-  ProviderSubscription<State> _listenElement<State>(
-    ProviderElementBase<State> element, {
-    required void Function(State? previous, State next) listener,
-    required void Function(Object error, StackTrace stackTrace) onError,
-  }) {
-    final sub = _ExternalProviderSubscription<State>._(
-      element,
-      listener,
-      onError: onError,
-    );
-
-    element._externalDependents.add(sub);
-
-    return sub;
-  }
-
   /// {@macro riverpod.listen}
   @override
   ProviderSubscription<State> listen<State>(
@@ -653,7 +636,7 @@ final b = Provider((ref) => ref.watch(a), dependencies: [a]);
   }
 
   /// Traverse the [ProviderElementBase]s associated with this [ProviderContainer].
-  Iterable<ProviderElementBase<Object?>> getAllProviderElements() sync* {
+  Iterable<ProviderElementBase> getAllProviderElements() sync* {
     for (final reader in _stateReaders.values) {
       if (reader._element != null && reader.container == this) {
         yield reader._element!;
@@ -666,9 +649,9 @@ final b = Provider((ref) => ref.watch(a), dependencies: [a]);
   /// This is fairly expensive and should be avoided as much as possible.
   /// If you do not need for providers to be sorted, consider using [getAllProviderElements]
   /// instead, which returns an unsorted list and is significantly faster.
-  Iterable<ProviderElementBase<Object?>> getAllProviderElementsInOrder() sync* {
-    final visitedNodes = HashSet<ProviderElementBase<Object?>>();
-    final queue = DoubleLinkedQueue<ProviderElementBase<Object?>>();
+  Iterable<ProviderElementBase> getAllProviderElementsInOrder() sync* {
+    final visitedNodes = HashSet<ProviderElementBase>();
+    final queue = DoubleLinkedQueue<ProviderElementBase>();
 
     // get providers that don't depend on other providers from this container
     for (final reader in _stateReaders.values) {