Race condition in popScopesTill

[svsk417]

Hello there!
I just observed a race condition in popScopesTillFunction when executed multiple times within the same async execution block (is it called this way?) of the run loop.
My scenario is, that I want to pop four scopes at once and the scopes are pushed as follows from inner to outer:

• ScopeA
• ScopeB
• ScopeC
• ScopeD

In my implementation I make sure that I pop the scopes from outer to inner - at least I call the async functions in the following order, but in different places. As such they might all get to poppedScopeName = _currentScope.name; in the implementation of the package. If now popScopesTill(ScopeA); discards a scope that the other popScopesTill should discard and the other popScopesTill does find its target scope to pop, the scopes will be popped until the root scope is reached the application will crash:

popScopesTill(ScopeD);
popScopesTill(ScopeC);
popScopesTill(ScopeB);
popScopesTill(ScopeA);

Current implementation:

  @override
  Future<bool> popScopesTill(String scopeName, {bool inclusive = true}) async {
    assert(
      scopeName != _baseScopeName || !inclusive,
      "You can't pop the base scope",
    );
    if (_scopes.firstWhereOrNull((x) => x.name == scopeName) == null) {
      return false;
    }
    String? poppedScopeName;
    do {
      poppedScopeName = _currentScope.name;
      await popScope();
    } while (inclusive
        ? (poppedScopeName != scopeName)
        : (_currentScope.name != scopeName));
    onScopeChanged?.call(false);
    return true;
  }

This could be prevented using the synchronized package and locking this function. What do you think about this solution?
Thank you for your time to maintain this package! We are really grateful for it! If I can support you with the implementation, just post me!

Implementation details of the way scoppes are pushed and popped

import 'package:blocs/blocs.dart';
import 'package:flutter/material.dart';
import 'package:flutter_bloc/flutter_bloc.dart';

import 'di.dart';

class DiScopeWidget extends StatelessWidget {
  final List<DiScope> diScopes;
  final Widget child;
  final Widget? loadingWidget;

  DiScopeWidget({
    required DiScope diScope,
    required this.child,
    this.loadingWidget,
    super.key,
  }) : diScopes = [diScope];

  DiScopeWidget.multipleScopes({
    required this.diScopes,
    required this.child,
    this.loadingWidget,
    super.key,
  }) : assert(diScopes.isNotEmpty);

  @override
  Widget build(BuildContext context) {
    final List<Widget> blocProviders = [];
    for (final (index, diScope) in diScopes.reversed.indexed) {
      // this is the inner bloc provider
      if (index == 0) {
        blocProviders.add(
          Builder(
            key: ValueKey('${diScope.toString()}_Builder'),
            builder: (context) {
              return BlocProvider(
                key: ValueKey('${diScope.toString()}_BlocProvider'),
                create: (context) => getIt<DiScopeCubit>(
                  param1: diScope,
                ),
                child: BlocBuilder<DiScopeCubit, DiScopeState>(
                  key: ValueKey('${diScope.toString()}_BlocBuilder'),
                  builder: (context, state) {
                    return state.when(
                      loading: () => loadingWidget ?? Container(),
                      ready: () => child,
                    );
                  },
                ),
              );
            },
          ),
        );
        continue;
      }
      blocProviders.add(
        Builder(
          key: ValueKey('${diScope.toString()}_Builder'),
          builder: (context) {
            return BlocProvider(
              key: ValueKey('${diScope.toString()}_BlocProvider'),
              create: (context) => getIt<DiScopeCubit>(
                param1: diScope,
              ),
              child: BlocBuilder<DiScopeCubit, DiScopeState>(
                key: ValueKey('${diScope.toString()}_BlocBuilder'),
                builder: (context, state) {
                  return state.when(
                    loading: () => loadingWidget ?? Container(),
                    ready: () => blocProviders[index - 1],
                  );
                },
              ),
            );
          },
        ),
      );
    }
    return blocProviders.last;
  }
}

class DiScopeCubit extends Cubit<DiScopeState> {
  final DiScope _diScope;
  final ScopesHandler _scopesHandler;

  DiScopeCubit({
    required DiScope diScope,
    required ScopesHandler scopesHandler,
  })  : _diScope = diScope,
        _scopesHandler = scopesHandler,
        super(const DiScopeState.loading()) {
    switch (diScope) {
        // push scopes
        .then(() => emit(DiScopeState.ready()));
    }
  }

  @override
  Future<void> close() async {
    switch (_diScope) {
        // popScopesUntil the specified scope
    }
    await super.close();
  }
}

[svsk417]

Relates to #364

[escamoteur]

Will look into it. Did you check the latest prereleases if it still happens there? Am 9. Aug. 2024, 08:04 +0100 schrieb svsk417 ***@***.***>:

…

Relates to #364 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[svsk417]

Yes, I checked it. Happens also there. Thank you for looking into it.
I guess I can use dropScope in my case as well, but just wanted to notify you that there is an issue. So, it is not that urgent.

[escamoteur]

Just to clarify the problem is that one popScopeTil already removes a scope the other popScopeTil has as the scope where it should stop? I see that this could happen but why do you fire more than one popScopeTil in your app? The other question is should I enforce that a second call to popScopeTil waits until the first one finished? Or should I add an assert as guard that you get a direct feedback that you are running into this problem? Am 9. Aug. 2024, 07:29 +0100 schrieb svsk417 ***@***.***>:

…

Hello there! I just observed a race condition in popScopesTillFunction when executed multiple times within the same async execution block (is it called this way?) of the run loop. My scenario is, that I want to pop four scopes at once and the scopes are pushed as follows from inner to outer: • ScopeA • ScopeB • ScopeC • ScopeD In my implementation I make sure that I pop the scopes from outer to inner - at least I call the async functions in the following order, but in different places. As such they might all get to poppedScopeName = _currentScope.name; in the implementation of the package. If now popScopesTill(ScopeA); discards a scope that the other popScopesTill should discard and the other popScopesTill does find its target scope to pop, the scopes will be popped until the root scope is reached the application will crash: popScopesTill(ScopeD); popScopesTill(ScopeC); popScopesTill(ScopeB); popScopesTill(ScopeA); Current implementation: @OverRide Future<bool> popScopesTill(String scopeName, {bool inclusive = true}) async { assert( scopeName != _baseScopeName || !inclusive, "You can't pop the base scope", ); if (_scopes.firstWhereOrNull((x) => x.name == scopeName) == null) { return false; } String? poppedScopeName; do { poppedScopeName = _currentScope.name; await popScope(); } while (inclusive ? (poppedScopeName != scopeName) : (_currentScope.name != scopeName)); onScopeChanged?.call(false); return true; } This could be prevented using the synchronized package and locking this function. What do you think about this solution? Thank you for your time to maintain this package! We are really grateful for it! If I can support you with the implementation, just post me! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[svsk417]

Yes, if you take a look at the image and use the code that I have provided in 'Implementation details of the way scoppes are pushed and popped', the widgets will be discarded in the following order, if the user logs out:

1. Payment -> PaymentScope
2. Cart -> Nothing
3. ShoppingOverview -> CartScope & LoggedInScope

IMO this could be a valid case
Yes, I would lock it and wait for the scope to be popped. But the issue I see in here, would be that resolution of a dependency after/while the scope is popped might yield a service that is about to be disposed. Therefore, the lock would need to be a read/write lock, instead of just an exclusive access (write -> locking other write & read ops) and read (only locking write ops while reading). But this might be not that advisable, since not every resolution is async. 🤔 Maybe that is Ok, since we are not operating in an environment where every loop needs to be verified. Was just thinking about it. Thank you for looking into this!

[escamoteur]

Have you considered using watch_it for pushing scopes that are related to widgets? Am 12. Aug. 2024, 08:26 +0100 schrieb svsk417 ***@***.***>:

…

grafik.png (view on web) Yes, if you take a look at the image and use the code that I have provided in 'Implementation details of the way scoppes are pushed and popped', the widgets will be discarded in the following order, if the user logs out: 1. Payment -> PaymentScope 2. Cart -> Nothing 3. ShoppingOverview -> CartScope & LoggedInScope IMO this could be a valid case Yes, I would lock it and wait for the scope to be popped. But the issue I see in here, would be that resolution of a dependency after/while the scope is popped might yield a service that is about to be disposed. Therefore, the lock would need to be a read/write lock, instead of just an exclusive access (write -> locking other write & read ops) and read (only locking write ops while reading). But this might be not that advisable, since not every resolution is async. 🤔 Maybe that is Ok, since we are not operating in an environment where every loop needs to be verified. Was just thinking about it. Thank you for looking into this! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[escamoteur]

When you say read and write lock? What would be the write use case? That a new scope is pushed at the same time? For making pushing save I an not await it because it's not an async function so there the guard is a hard assert. So I don't how I could deal with that whole popping the scopes. Am 12. Aug. 2024, 08:26 +0100 schrieb svsk417 ***@***.***>:

…

grafik.png (view on web) Yes, if you take a look at the image and use the code that I have provided in 'Implementation details of the way scoppes are pushed and popped', the widgets will be discarded in the following order, if the user logs out: 1. Payment -> PaymentScope 2. Cart -> Nothing 3. ShoppingOverview -> CartScope & LoggedInScope IMO this could be a valid case Yes, I would lock it and wait for the scope to be popped. But the issue I see in here, would be that resolution of a dependency after/while the scope is popped might yield a service that is about to be disposed. Therefore, the lock would need to be a read/write lock, instead of just an exclusive access (write -> locking other write & read ops) and read (only locking write ops while reading). But this might be not that advisable, since not every resolution is async. 🤔 Maybe that is Ok, since we are not operating in an environment where every loop needs to be verified. Was just thinking about it. Thank you for looking into this! — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[svsk417]

I have not, yet. Thanks for the suggestion!
Write use case would be

1. Preventing that dependencies are registered while a scope is being popped (if not done prevented yet!?)
2. Preventing the original issue with popScopeTil
3. Preventing that dependencies are resolved while a "popping process" is executing (if not done prevented yet!?)

[escamoteur]

Yeah, that is all a bit tricky, I blocked that pushing is possible. You are correct that 3 could be an issue in certain situations but to prevent that all get calls would need to be async. Just wondering, do you await the popScopeTil calls? If you don't push the scopes with watch_it I think it is a general problem to use popScopeTil in a dispose if that happens multiple times while popping a page stack. That's why I in general prefer doing pushing and popping of scopes in the business logic. One other approach could be that popScopeTil checks first if the til-scope is still there after execution of the async disposal and if not that it then just drops the scope it was called on Am 12. Aug. 2024, 08:37 +0100 schrieb svsk417 ***@***.***>:

…

I have not, yet. Thanks for the suggestion! Write use case would be 1. Preventing that dependencies are registered while a scope is being popped (if not done prevented yet!?) 2. Preventing the original issue with popScopeTil 3. Preventing that dependencies are resolved while a "popping process" is executing (if not done prevented yet!?) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[escamoteur]

One more 😉 such a lock would need to be reentrant so I would need to really use something like the synchronized package and I honestly don't want to add such a biest to get_it. I will add the save guard to the popping loop and let's see if that already solves your problem. Am 12. Aug. 2024, 08:37 +0100 schrieb svsk417 ***@***.***>:

…

I have not, yet. Thanks for the suggestion! Write use case would be 1. Preventing that dependencies are registered while a scope is being popped (if not done prevented yet!?) 2. Preventing the original issue with popScopeTil 3. Preventing that dependencies are resolved while a "popping process" is executing (if not done prevented yet!?) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[escamoteur]

Pleast test 8.0.0-pre-7 if you still ahve that problem

[svsk417]

I just tested it. It seems to work with 8.0.0-pre-7.
Thanks for the effort!