Skip to content

Stack Overflow error when two classes depend on each other #82

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

Closed
ben256 opened this issue May 23, 2020 · 11 comments
Closed

Stack Overflow error when two classes depend on each other #82

ben256 opened this issue May 23, 2020 · 11 comments

Comments

@ben256
Copy link

ben256 commented May 23, 2020

I have an Auth Service and a firebase service which are both registered as singletons and usually everything works fine. However, when I try and access the Auth service from firebase service and vice versa, eg. AuthService(FirebaseService()) <-> FirebaseService(AuthService()), I get a stack overflow error with the full error trace as follows:

flutter: ══╡ EXCEPTION CAUGHT BY WIDGETS LIBRARY ╞═══════════
flutter: The following StackOverflowError was thrown building StartupView:
flutter: Stack Overflow
flutter:
flutter: The relevant error-causing widget was:
flutter:   StartupView file:///Users/ben/IdeaProjects/testapp/lib/app/router.gr.dart:47:33
flutter:
flutter: When the exception was thrown, this was the stack:
flutter: #0      printToConsole (dart:_internal-patch/print_patch.dart:13:1)
flutter: #4      print (dart:core/print.dart:15:16)
flutter: #5      _ServiceFactory.getObject (package:get_it/get_it_impl.dart:137:7)
flutter: #6      _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #7      _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #8      new AuthenticationService (package: testapp/services/authentication_service.dart:11:53)
flutter: #9      _$ThirdPartyServicesModule.authenticationService (package: testapp/app/locator.iconfig.dart:27:54)
flutter: #10     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:16:38)
flutter: #11     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: #12     _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #13     _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #14     new FirestoreService (package: testapp/services/firestore_service.dart:11:10)
flutter: #15     _$ThirdPartyServicesModule.firestoreService (package: testapp/app/locator.iconfig.dart:31:44)
flutter: #16     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:20:38)
flutter: #17     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: #18     _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #19     _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #20     new AuthenticationService (package: testapp/services/authentication_service.dart:11:53)
flutter: #21     _$ThirdPartyServicesModule.authenticationService (package: testapp/app/locator.iconfig.dart:27:54)
flutter: #22     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:16:38)
flutter: #23     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: #24     _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #25     _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #26     new FirestoreService (package: testapp/services/firestore_service.dart:11:10)
flutter: #27     _$ThirdPartyServicesModule.firestoreService (package: testapp/app/locator.iconfig.dart:31:44)
flutter: #28     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:20:38)
flutter: #29     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: #30     _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #31     _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #32     new AuthenticationService (package: testapp/services/authentication_service.dart:11:53)
flutter: #33     _$ThirdPartyServicesModule.authenticationService (package: testapp/app/locator.iconfig.dart:27:54)
flutter: #34     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:16:38)
flutter: #35     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: #36     _GetItImplementation.get (package:get_it/get_it_impl.dart:276:34)
flutter: #37     _GetItImplementation.call (package:get_it/get_it_impl.dart:288:12)
flutter: #38     new FirestoreService (package: testapp/services/firestore_service.dart:11:10)
flutter: #39     _$ThirdPartyServicesModule.firestoreService (package: testapp/app/locator.iconfig.dart:31:44)
flutter: #40     $initGetIt.<anonymous closure> (package: testapp/app/locator.iconfig.dart:20:38)
flutter: #41     _ServiceFactory.getObject (package:get_it/get_it_impl.dart:128:40)
flutter: ...
flutter: ...
flutter: ...     Normal element mounting (21 frames)
flutter: #3747   Element.inflateWidget (package:flutter/src/widgets/framework.dart:3446:14)
flutter: #3748   Element.updateChild (package:flutter/src/widgets/framework.dart:3214:18)
flutter: #3749   RenderObjectToWidgetElement._rebuild (package:flutter/src/widgets/binding.dart:1148:16)
flutter: #3750   RenderObjectToWidgetElement.mount (package:flutter/src/widgets/binding.dart:1119:5)
flutter: #3751   RenderObjectToWidgetAdapter.attachToRenderTree.<anonymous closure> (package:flutter/src/widgets/binding.dart:1061:17)
flutter: #3752   BuildOwner.buildScope (package:flutter/src/widgets/framework.dart:2607:19)
flutter: #3753   RenderObjectToWidgetAdapter.attachToRenderTree (package:flutter/src/widgets/binding.dart:1060:13)
flutter: #3754   WidgetsBinding.attachRootWidget (package:flutter/src/widgets/binding.dart:941:7)
flutter: #3755   WidgetsBinding.scheduleAttachRootWidget.<anonymous closure> (package:flutter/src/widgets/binding.dart:922:7)
flutter: (elided 14 frames from class _RawReceivePortImpl, class _Timer, dart:async, and dart:async-patch)
flutter:
flutter: ════════════════════════════════════════════

The modules were registered using injectable, so from this:

final locator = GetIt.instance;

@injectableInit
void setupLocator() => $initGetIt(locator);

It generated this:

void $initGetIt(GetIt g, {String environment}) {
  final thirdPartyServicesModule = _$ThirdPartyServicesModule();
  g.registerLazySingleton<AuthenticationService>(
      () => thirdPartyServicesModule.authenticationService);
  g.registerLazySingleton<DialogService>(
      () => thirdPartyServicesModule.dialogService);
  g.registerLazySingleton<FirestoreService>(
      () => thirdPartyServicesModule.firestoreService);
  g.registerLazySingleton<NavigationService>(
      () => thirdPartyServicesModule.navigationService);
}

class _$ThirdPartyServicesModule extends ThirdPartyServicesModule {
  @override
  AuthenticationService get authenticationService => AuthenticationService();
  @override
  DialogService get dialogService => DialogService();
  @override
  FirestoreService get firestoreService => FirestoreService();
  @override
  NavigationService get navigationService => NavigationService();
}

I'm fairly sure my code isn't causing the problem as deleting the body of the services, then running leads to the same error. But, if this is meant to happen, is there anyway I can work around it and still access the each service in the other service.
Thanks

@ben256
Copy link
Author

ben256 commented May 25, 2020

Sorry, this was just me being oblivious to circular dependencies, nothing wrong with the package

@pierre-luc
Copy link

GetIt should give us some details about circular dependency.

To solve this problem here a part solution with this example:

Some interfaces and implementations:

abstract class SuperLetter {}

abstract class A implements SuperLetter {}

class AC implements A {
  final B b;
  AC({this.b});

  @override
  String toString() {
    if (b == null) {
      return "Im an A.";
    }
    return "Im an A and my B is: $b";
  }
}

abstract class B implements SuperLetter {}

class BC implements B {
  final C c;
  BC({this.c});

  @override
  String toString() {
    if (c == null) {
      return "Im a B.";
    }
    return "Im a B and my C: $c";
  }
}

abstract class C implements SuperLetter {}

class CC implements C {

  final SuperLetter a;
  CC({this.a});

  @override
  String toString() {
    if (a == null) {
      return "Im a C.";
    }
    return "Im a C and my A: $a"; // might never be reached because of circular dependency
  }
}

A new Exception to throw when circular dependency is detected.

class CircularDependencyException implements Exception {
  final wantedType;
  final calledTypes;

  CircularDependencyException({
    this.wantedType,
    this.calledTypes,
  });

  @override
  String toString() {
    String message = '🔺 Circular dependency ';
    for (final e in calledTypes) {
      message += '${e.toString()}';
      if (e == calledTypes.last) {
        message += ' -> $wantedType';
      } else {
        message += ' -> ';
      }
    }
    return message;
  }
}

To fight against circular dependency:

class GetItProtector {
  var _wantedType;
  Set _calledTypes;
  final GetIt getIt;

  GetItProtector(this.getIt);

  _check<T>() {
    if (_wantedType == null) {
      _wantedType = T;
      _calledTypes = Set()..add(T);
    } else {
      if (_calledTypes.contains(T)) {
        throw CircularDependencyException(wantedType: T, calledTypes: _calledTypes);
      }
      _calledTypes.add(T);
    }
  }

  T call<T>() {
    _check<T>();
    final instance = getIt<T>();
    if (instance.runtimeType == _wantedType) {
      _wantedType = null;
      _calledTypes.clear();
    }
    return instance;
  }
}

Example with A -> B -> C -> A circular dependency:

void main() {
  GetIt g = GetIt.instance;
  final myGet = GetItProtector(g);
  g.registerLazySingleton<A>(() => AC(
        b: myGet(),
      ));

  g.registerLazySingleton<B>(() => BC(
        c: myGet(),
      ));

  g.registerLazySingleton<C>(() => CC(
        a: myGet<A>(),
      ));

  A a = myGet();
  print(a);
}

Output:

Error while creating C
Stack trace: ......

Error while creating B
Stack trace: ......

Error while creating A
Stack trace: ......

Unhandled exception:
🔺 Circular dependency A -> B -> C -> A
....

Example with A -> B -> C -> B

void main() {
  GetIt g = GetIt.instance;
  final myGet = GetItProtector(g);
  g.registerLazySingleton<A>(() => AC(
        b: myGet(),
      ));

  g.registerLazySingleton<B>(() => BC(
        c: myGet(),
      ));

  g.registerLazySingleton<C>(() => CC(
        a: myGet<B>(),
      ));

  A a = myGet();
  print(a);
}

Output

Error while creating C
Stack trace: ......

Error while creating B
Stack trace: ......

Error while creating A
Stack trace: ......

Unhandled exception:
🔺 Circular dependency A -> B -> C -> B
....

In this solution I don't test if type is a real type and not Object or dynamic.

@escamoteur
Copy link
Collaborator

I honestly don't understand how
image
this ever can be true because your wantedtype is always an abstract type and the runtimetype is always a derived class.

@escamoteur escamoteur reopened this Oct 6, 2020
@pierre-luc
Copy link

You are right. We could use an increment to each call and decrement it after an instance is returned from getIt. In _check method if _wantedType is null we init _depth to 0.

T call<T>() {
    _check<T>();
    _depth += 1;
    final instance = getIt<T>();
    _depth -= 1;
    if (_depth == 0) {
      _wantedType = null;
      _calledTypes.clear();
    }
    return instance;
  }

@escamoteur
Copy link
Collaborator

Haven't heard many people complaining about this problem. Have to think how this could be avoided inside get_it

@pr-1
Copy link

pr-1 commented Oct 6, 2020

I am also facing same issue and it is a pain.

@Abion47
Copy link

Abion47 commented Oct 6, 2020

I'm going to say the same thing here as I said on pierre-luc's StackOverflow post on this topic.

There's not going to be any way that GetIt can automatically detect circular dependencies without one of two things: an interface for types that get registered that the end developer will be responsible with implementing, or reflection. The latter is a non-starter since Flutter doesn't support it, and the former requires that developers A) know about the feature and B) are willing to put in the work to add the boilerplate into every service class they register. And even then, this is not a solution, but merely a polite warning that there is a problem.

There is no real solution to circular dependency, which is fine because it is the result of bad coding practices (tight coupling). The best way to "fix" it is to not have it in the first place. With OP's code, for example, the solution is to not use the constructor to pass in the dependency but instead retrieve it through GetIt or another dependency injection method (such as thunks or lazy evaluation) separate from the constructor.

Sure it could be nice to get a heads up if you inadvertently create a circular dependency, but pierre's solution is a lot of work for relatively little gain. The normal detection method of getting stack overflow errors is less clear but no less apparent in indicating that a problem exists, and all you would need to do at that point is to step through your program and you will immediately spot the issue.

@escamoteur
Copy link
Collaborator

@Abion47 thanks for your comment. I think this exactly what I think!

@tomasbaran
Copy link

I probably simplified the issue here: https://stackoverflow.com/questions/71223255/getit-plugin-stack-overflow-error-when-two-classes-depend-on-each-other

@escamoteur Can you please comment what the correct solution would be?

@Abion47
Copy link

Abion47 commented Feb 22, 2022

@tomasbaran The correct solution is to not have a circular dependency at all. And to be perfectly honest, two classes that have a tightly-coupled dependency on each other is code smell anyway. If you are running into this issue, it's a good sign that you need to reconsider your architecture.

@escamoteur
Copy link
Collaborator

I totally agree. I also just added a comment to the SO.

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

No branches or pull requests

6 participants