Race condition when calling a side-effect of an AutoDisposeAsyncNotifierProvider

[AhmedLSayed9]

Discussed in #2502

Describe the bug

void main() async {
  final container = ProviderContainer();
  await container.read(sampleProvider.notifier).fetch();
}

@riverpod
class Sample extends _$Sample {
  @override
  FutureOr<int> build() => 0;

  Future<void> fetch() async {
    state = const AsyncLoading();
    await Future.delayed(const Duration(seconds: 1));
    state = const AsyncData(0);
  }
}

Running this will fail with an exception: Bad state: Future already completed

What happens is:
1- The provider is initialized synchronously with 0
2- state = const AsyncLoading(); get executed and initialize _futureCompleter
3- The provider get disposed after that (no listeners): dispose() method check and find that _futureCompleter has value but at the same time _lastFuture is null (as the provider is synchronously initialized), so it completes _futureCompleter with an error.
4- state = const AsyncData(0); executes after than and find _futureCompleter is not null so it completes it (again). Maybe _futureCompleter should be set to null after completing it in the dispose method to fix this?

I'm curious where's the other part of completing _futureCompleter when _lastFuture is not null (commented here)

Expected behavior
I except the code to run with no exception as the current behavior is suppressing the setter of a disposed provider without throwing an exception.

[rrousselGit]

Your provider got disposed, so it shouldn't be calling state= anymore.

The error isn't the most user-friendly one, your state = const AsyncData(0); isn't valid in your scenario.
That's the same problem as the "don't use build_context across async gap".

You should handle the fact that the provider was canceled within the operation and act accordingly. To begin with, the pattern you've implemented in your notifier (build doing nothing and the fetching being done in a side-effecf) is discouraged. Have build do the fetching instead. If you don't want to fetch immediately, don't read the provider immediately.

[AhmedLSayed9]

Your provider got disposed, so it shouldn't be calling state= anymore.

I agree on that. but the error is just confusing as it's not related to using state setter. I was thinking this should work fine as the current behavior of using state = after a provider get disposed doesn't throw.

[rrousselGit]

I was thinking this should work fine as the current behavior of using state = after a provider get disposed doesn't throw.

If you found a case where it doesn't throw, then that's a bug. It should.

The error should be changed to something more reasonable, but you should definitely get an error.

[AhmedLSayed9]

I think currently using state = for a disposed provider doesn't throw at all:

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

Line 203 in 8bcf25f

// TODO assert Notifier isn't disposed

It's just ignored as we're checking for mounted before updating the state

[rrousselGit]

Ah well, my bad ha
Anyway it should throw

[AhmedLSayed9]

Removing this line from my sample above will let it work:

state = const AsyncLoading();

Adding it causes the issue because it initializes _futureCompleter again which will then be updated to be completed by dispose method (without setting it to null). Using a setter after that will throw as it checks only if _futureCompleter is not null but not completed:

if (completer != null) {
      completer.complete(value.value);
      _futureCompleter = null;
    }

[AhmedLSayed9]

Ah well, my bad ha
Anyway it should throw

Alright! no problem. but that mean we should be able to check for mounted like with BuildContext?

[rrousselGit]

Check ref.onDispose

[samderlust]

I have the same issue. the provider is disposed while state = await AsyncValue.guard is running.
so Unhandled Exception: Bad state: Future already completed was thrown.
Any practical solution for this?

[AhmedLSayed9]

I have the same issue. the provider is disposed while state = await AsyncValue.guard is running.
so Unhandled Exception: Bad state: Future already completed was thrown.
Any practical solution for this?

As Remi said you should check if the provider is disposed before setting the state. something like checking mounted for BuildContext.
You might want to check #1879 for ideas on how to do that.

[samderlust]

@AhmedLSayed9 Thank you, it's a bit cumbersome but it works for me.

[rrousselGit]

state= throws now if called on a disposed notifier. So closing