diff --git a/README.md b/README.md index 7a122ca..9f092a4 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,9 @@ Choose the lifetime that fits your needs: - **Factory** — New instance every time. Great for stateless services or objects with short lifetimes. [Read more →](https://flutter-it.dev/documentation/get_it/object_registration#factory) +> **Avoid untyped tear-offs as factories.** Registering `getIt.call` (or an untyped `getIt.get` tear-off) as `registerLazySingleton(...)` re-enters `get()` and used to overflow the stack. GetIt now throws a clear `StateError` for that circular self-resolution. Prefer `() => getIt()` or a named function that keeps the concrete type argument. + + ### Advanced Features - **Scopes** — Create hierarchical registration scopes for different app states (login/logout, sessions, feature flags). diff --git a/lib/get_it_impl.dart b/lib/get_it_impl.dart index db09764..599bec3 100644 --- a/lib/get_it_impl.dart +++ b/lib/get_it_impl.dart @@ -99,6 +99,11 @@ class _ObjectRegistration /// they are stored here final List objectsWaiting = []; + /// True while [creationFunction] / async factory for this registration is + /// running. Used to fail fast on circular self-resolution (e.g. registering + /// `getIt.call` as a factory for `T`, which re-enters `get()`). + bool _creationInProgress = false; + @override bool get isReady => _readyCompleter.isCompleted; @@ -213,6 +218,47 @@ class _ObjectRegistration } } + Never _throwCircularSelfResolution() { + final named = + instanceName != null ? ' (instanceName: "$instanceName")' : ''; + throw StateError( + 'GetIt: Circular dependency detected while creating $registeredWithType$named.\n' + 'This often happens when registering with an untyped tear-off of get/call, e.g.:\n' + ' getIt.registerLazySingleton(getIt.call);\n' + 'which re-enters get<$registeredWithType>() and recurses until StackOverflowError.\n' + 'Use an explicit factory that constructs or resolves a concrete type, e.g.:\n' + ' getIt.registerLazySingleton(() => getIt());\n' + 'or a named function tear-off that keeps the concrete type argument.', + ); + } + + T _runCreation(T Function() create) { + if (_creationInProgress) { + _throwCircularSelfResolution(); + } + _creationInProgress = true; + try { + return create(); + } finally { + _creationInProgress = false; + } + } + + /// Guards only the synchronous call that starts creation. Cleared when the + /// factory returns a [Future], not when that Future completes — so overlapping + /// [getAsync] on always-new / cached factories stay valid. + Future _runCreationAsync(Future Function() create) { + if (_creationInProgress) { + _throwCircularSelfResolution(); + } + _creationInProgress = true; + try { + return create(); + } finally { + _creationInProgress = false; + } + } + /// returns an instance depending on the type of the registration if [async==false] T getObject(dynamic param1, dynamic param2) { assert( @@ -230,9 +276,11 @@ class _ObjectRegistration if (creationFunctionParam != null) { // Validate parameters in debug mode _validateFactoryParams(param1, param2); - return creationFunctionParam!(param1 as P1, param2 as P2); + return _runCreation( + () => creationFunctionParam!(param1 as P1, param2 as P2), + ); } else { - return creationFunction!(); + return _runCreation(creationFunction!); } case ObjectRegistrationType.cachedFactory: if (weakReferenceInstance?.target != null && @@ -240,16 +288,17 @@ class _ObjectRegistration param2 == lastParam2) { return weakReferenceInstance!.target!; } else { - T newInstance; - if (creationFunctionParam != null) { - // Validate parameters in debug mode BEFORE casting - _validateFactoryParams(param1, param2); - lastParam1 = param1 as P1?; - lastParam2 = param2 as P2?; - newInstance = creationFunctionParam!(param1 as P1, param2 as P2); - } else { - newInstance = creationFunction!(); - } + final T newInstance = _runCreation(() { + if (creationFunctionParam != null) { + // Validate parameters in debug mode BEFORE casting + _validateFactoryParams(param1, param2); + lastParam1 = param1 as P1?; + lastParam2 = param2 as P2?; + return creationFunctionParam!(param1 as P1, param2 as P2); + } else { + return creationFunction!(); + } + }); weakReferenceInstance = WeakReference(newInstance); return newInstance; } @@ -257,15 +306,18 @@ class _ObjectRegistration return instance!; case ObjectRegistrationType.lazy: if (instance == null) { - if (useWeakReference) { - if (weakReferenceInstance != null) { - /// this means that the instance was already created and disposed - _readyCompleter = Completer(); + _runCreation(() { + if (useWeakReference) { + if (weakReferenceInstance != null) { + /// this means that the instance was already created and disposed + _readyCompleter = Completer(); + } + weakReferenceInstance = WeakReference(creationFunction!()); + } else { + _instance = creationFunction!(); } - weakReferenceInstance = WeakReference(creationFunction!()); - } else { - _instance = creationFunction!(); - } + return instance!; + }); objectsWaiting.clear(); _readyCompleter.complete(); @@ -290,8 +342,14 @@ class _ObjectRegistration return instance!; } } catch (e, s) { - _debugOutput('Error while creating $T'); - _debugOutput('Stack trace:\n $s'); + // Only build/print the stack when debug output is enabled. Eagerly + // interpolating `$s` can amplify StackOverflowError into native crashes. + if (_isDebugMode && !GetIt.noDebugOutput) { + // ignore: avoid_print + print('Error while creating $T'); + // ignore: avoid_print + print('Stack trace:\n $s'); + } rethrow; } } @@ -321,10 +379,11 @@ class _ObjectRegistration if (asyncCreationFunctionParam != null) { // Validate parameters in debug mode _validateFactoryParams(param1, param2); - return asyncCreationFunctionParam!(param1 as P1, param2 as P2) - as Future; + return _runCreationAsync( + () => asyncCreationFunctionParam!(param1 as P1, param2 as P2), + ) as Future; } else { - return asyncCreationFunction!() as Future; + return _runCreationAsync(asyncCreationFunction!) as Future; } case ObjectRegistrationType.cachedFactory: if (weakReferenceInstance?.target != null && @@ -332,24 +391,26 @@ class _ObjectRegistration param2 == lastParam2) { return Future.value(weakReferenceInstance!.target! as R); } else { - if (asyncCreationFunctionParam != null) { - // Validate parameters in debug mode BEFORE casting - _validateFactoryParams(param1, param2); - lastParam1 = param1 as P1?; - lastParam2 = param2 as P2?; - return asyncCreationFunctionParam!( - param1 as P1, - param2 as P2, - ).then((value) { - weakReferenceInstance = WeakReference(value); - return value; - }) as Future; - } else { - return asyncCreationFunction!().then((value) { - weakReferenceInstance = WeakReference(value); - return value; - }) as Future; - } + return _runCreationAsync(() { + if (asyncCreationFunctionParam != null) { + // Validate parameters in debug mode BEFORE casting + _validateFactoryParams(param1, param2); + lastParam1 = param1 as P1?; + lastParam2 = param2 as P2?; + return asyncCreationFunctionParam!( + param1 as P1, + param2 as P2, + ).then((value) { + weakReferenceInstance = WeakReference(value); + return value; + }); + } else { + return asyncCreationFunction!().then((value) { + weakReferenceInstance = WeakReference(value); + return value; + }); + } + }) as Future; } case ObjectRegistrationType.constant: if (instance != null) { @@ -369,47 +430,63 @@ class _ObjectRegistration return pendingResult! as Future; } - /// Seems this is really the first access to this async Singleton - final asyncResult = asyncCreationFunction!(); - - pendingResult = asyncResult.then((newInstance) { - if (!shouldSignalReady) { - /// only complete automatically if the registration wasn't marked with - /// [signalsReady==true] - _readyCompleter.complete(); - objectsWaiting.clear(); - } - if (useWeakReference) { - weakReferenceInstance = WeakReference(newInstance); - } else { - _instance = newInstance; - } + if (_creationInProgress) { + _throwCircularSelfResolution(); + } + _creationInProgress = true; - // Call onCreated callback if provided - onCreatedCallback?.call(newInstance); - - /// check if we are shadowing an existing Object - final registrationThatWouldbeShadowed = - _getItInstance._findFirstRegistrationByNameAndTypeOrNull( - instanceName, - type: T, - lookInScopeBelow: true, - ); - - final objectThatWouldbeShadowed = - registrationThatWouldbeShadowed?.instance; - if (objectThatWouldbeShadowed != null && - objectThatWouldbeShadowed is ShadowChangeHandlers) { - objectThatWouldbeShadowed.onGetShadowed(instance!); - } - return newInstance; - }); - return pendingResult! as Future; + /// Seems this is really the first access to this async Singleton + try { + final asyncResult = asyncCreationFunction!(); + + pendingResult = asyncResult.then((newInstance) { + if (!shouldSignalReady) { + /// only complete automatically if the registration wasn't marked with + /// [signalsReady==true] + _readyCompleter.complete(); + objectsWaiting.clear(); + } + if (useWeakReference) { + weakReferenceInstance = WeakReference(newInstance); + } else { + _instance = newInstance; + } + + // Call onCreated callback if provided + onCreatedCallback?.call(newInstance); + + /// check if we are shadowing an existing Object + final registrationThatWouldbeShadowed = + _getItInstance._findFirstRegistrationByNameAndTypeOrNull( + instanceName, + type: T, + lookInScopeBelow: true, + ); + + final objectThatWouldbeShadowed = + registrationThatWouldbeShadowed?.instance; + if (objectThatWouldbeShadowed != null && + objectThatWouldbeShadowed is ShadowChangeHandlers) { + objectThatWouldbeShadowed.onGetShadowed(instance!); + } + return newInstance; + }).whenComplete(() { + _creationInProgress = false; + }); + return pendingResult! as Future; + } catch (_) { + _creationInProgress = false; + rethrow; + } } } } catch (e, s) { - _debugOutput('Error while creating $T}'); - _debugOutput('Stack trace:\n $s'); + if (_isDebugMode && !GetIt.noDebugOutput) { + // ignore: avoid_print + print('Error while creating $T'); + // ignore: avoid_print + print('Stack trace:\n $s'); + } rethrow; } } diff --git a/test/circular_self_resolution_test.dart b/test/circular_self_resolution_test.dart new file mode 100644 index 0000000..4f67021 --- /dev/null +++ b/test/circular_self_resolution_test.dart @@ -0,0 +1,143 @@ +import 'package:get_it/get_it.dart'; +import 'package:test/test.dart'; + +abstract class Iface {} + +class Impl implements Iface {} + +void main() { + final getIt = GetIt.instance; + + setUp(() async { + await getIt.reset(); + GetIt.noDebugOutput = true; + }); + + tearDown(() async { + await getIt.reset(); + }); + + test( + 'lazy singleton registered with getIt.call throws instead of StackOverflow', + () { + getIt.registerLazySingleton(getIt.call); + + expect( + () => getIt(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Circular dependency detected'), + ), + ), + ); + }, + ); + + test( + 'lazy singleton factory that re-gets same type throws circular error', + () { + getIt.registerLazySingleton(() => getIt()); + + expect( + () => getIt(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Circular dependency detected'), + ), + ), + ); + }, + ); + + test( + 'lazy singleton alias to concrete impl resolves without circular error', + () { + getIt.registerLazySingleton(Impl.new); + getIt.registerLazySingleton(() => getIt()); + + final Iface instance = getIt(); + expect(instance, isA()); + expect(identical(instance, getIt()), isTrue); + }, + ); + + test( + 'factory registered with getIt.call throws instead of StackOverflow', + () { + getIt.registerFactory(getIt.call); + + expect( + () => getIt(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Circular dependency detected'), + ), + ), + ); + }, + ); + + test( + 'overlapping getAsync on same async factory succeeds', + () async { + var creates = 0; + getIt.registerFactoryAsync(() async { + creates++; + await Future.delayed(Duration.zero); + return Impl(); + }); + + final results = await Future.wait([ + getIt.getAsync(), + getIt.getAsync(), + ]); + + expect(results, hasLength(2)); + expect(results[0], isA()); + expect(results[1], isA()); + expect(creates, 2); + }, + ); + + test( + 'async factory that re-gets same type throws circular error', + () async { + getIt.registerFactoryAsync(() => getIt.getAsync()); + + await expectLater( + getIt.getAsync(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Circular dependency detected'), + ), + ), + ); + }, + ); + + test( + 'async lazy singleton registered with getIt.getAsync throws circular error', + () async { + getIt.registerLazySingletonAsync(() => getIt.getAsync()); + + await expectLater( + getIt.getAsync(), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Circular dependency detected'), + ), + ), + ); + }, + ); +}