Circular Dependencies causes crash

[jasperblues]

Steps to reproduce:

1. Create a login view controller:

• (id)loginController
  {
  return [TyphoonDefinition withClass:[MPLoginController class] initialization:^(TyphoonInitializer* initializer)
  {
  initializer.selector = @selector(initWithLoginView:);
  [initializer injectWithDefinition:[self loginView]];
  } properties:^(TyphoonDefinition* definition)
  {
  [definition injectProperty:@selector(authorizationUrl) withValueAsText:@"${oauth.authorization.url}"];
  [definition injectProperty:@selector(authentication) withDefinition:[self oauthAuthentication]];
  [definition injectProperty:@selector(keychainItemName) withValueAsText:@"${oauth.keychain.item.name}"];
  [definition injectProperty:@selector(loginView) withDefinition:[self loginView]];
  }];
  }
• (id)loginView
  {
  return [TyphoonDefinition withClass:[MPLoginView class] properties:^(TyphoonDefinition* definition)
  {
  [definition injectProperty:@selector(delegate) withDefinition:[self loginController]];
  }];
  }

1. Inject the loginViewController into a root view controller:

• (MPRootViewController_)rootViewController;
  {
  return (MPRootViewController_) [TyphoonDefinition withClass:[MPRootViewController class]
  initialization:^(TyphoonInitializer* initializer)
  {
  initializer.selector = @selector(initWithMainContentViewController:);
  [initializer injectWithDefinition:[self loginController]];
  } properties:^(TyphoonDefinition* definition)
  {
  definition.afterPropertyInjection = @selector(mplusInitialization);
  }];
  }

EXPECTED OUTCOME:

1. Works

ACTUAL OUTCOME

Causes crash.

[jasperblues]

Works if loginView is a singleton

[jasperblues]

Ok, for circular dependencies we

• Detect
• Let normal inject continue.
• Resolve the pre-built component at the end, finally injecting the circular dependency.

We resolve the pre-built component from the singleton cache, so its not going to work for prototypes. We need a temporary cache for prototypes.

• Should we allow multi-threading? This would mean the temporary cache has to be either synchonized or support multiple threads (maybe by generating a key). .

[cesteban]

Probably a dumb question (I didn't follow the STR yet), but: In the definition of loginController, why are you injecting loginView twice? (both in the initializer and by property).

[jasperblues]

@cesteban Oops, that was a mistake. . the problem still occurs in any case.

[cesteban]

I'm having similar issues with initializer injection on circular chains of prototypes. Typhoon gets trapped in infinite loop trying to instantiate the same prototype again and again.

I think we should register instances in _currentlyResolvingReferences earlier in the building process, right after allocating them. I reorganized the instance builder a little bit and everything seems to work better. I want to carefully test it. If everything goes right, I'll send a pull request.

Look forward to hear your thoughts on this.

[jasperblues]

Yes, its broken and waiting for a fix. . . I thought I checked and discovered that:

• When we're ready to inject circular deps, we get the built object by key.

This means that, if its a singleton, it will be ready and waiting for the circular deps to be provided. If its a prototype, getting by key, will kick-off creating a new one again. . . So instead we should put the circulars in a temp cache of their own, ready to retrieve and inject at the last step.

We may or may not want to allow multi-threaded access here. If we do, not only would we need a temp-cache, but an additional key to find the object again.

. . . I could be wrong here. But I think that's what's going on.

[cesteban]

Ok, I could be missing something, but in my mind the problem appears because in order to build MPLoginController Typhoon does the following:

1. Allocate an instance of MPLoginController
2. Invoke initializer on the instance (at this point instance is not registered in _currentlyResolvingReferences yet!)
3. Initializer needs a MPLoginView, this means: allocate, initialize, register in _currentlyResolvingReferences and inject properties
4. In this property injection phase, MPLoginController is not in _currentlyResolvingReferences yet, so it is not detected as circular dependency.
5. Typhoon will try to build another instance -> infinite loop

My solution: mark MPLoginController as currently resolving reference as soon as you have the instance allocated.

Makes sense?

I want to have time to thoroughly test it, but for the moment is working fine.