Skip to content
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

Circular prototypes #67

Merged
merged 8 commits into from
Sep 5, 2013
Merged

Conversation

cesteban
Copy link
Contributor

@cesteban cesteban commented Sep 5, 2013

This is the fix I was talking about in #57. The only important change is that registration in _currentlyResolvingReferences happens just after allocation of the instance. This solves the problem I was having with initializer injection and circular dependencies. Please, let me know your opinion.

I also added a very simple test that used to make Typhoon to fall in this well-known infinite loop. With this fix the test passes.

The other changes are basically reorganization of method buildInstanceWithDefinition:. Its multiple responsibilities (allocate instance, mark as currently resolving, initialize, resolve properties, mark as already resolved) are split in different methods now. I also renamed some methods to better reflect their responsibilities. This part is more readable now IMHO.

All feedback is greatly appreciated.

jasperblues added a commit that referenced this pull request Sep 5, 2013
@jasperblues jasperblues merged commit 3a24510 into appsquickly:master Sep 5, 2013
@jasperblues
Copy link
Member

Thanks Cesar! :) Much appreciated.

@jasperblues
Copy link
Member

Pushed this to main CocoaPods repo as 1.2.9

@jasperblues
Copy link
Member

Works like a charm in an external project. However, I'm trying to work out why the Tests crash now. Any idea?

(I badly need to fix the Bamboo build).

@jasperblues
Copy link
Member

- (void)setUp
{
    _componentFactory = [[TyphoonBlockComponentFactory alloc] initWithAssembly:[MiddleAgesAssembly assembly]];
    TyphoonPropertyPlaceholderConfigurer* configurer = [[TyphoonPropertyPlaceholderConfigurer alloc] init];
    [configurer usePropertyStyleResource:[TyphoonBundleResource withName:@"SomeProperties.properties"]];
    [_componentFactory attachMutator:configurer];

    //SOMETHING IN HERE IS CAUSING A CRASH IN THE TESTS  . . . (AppCode gives better reporting)
    _exceptionTestFactory = [[TyphoonBlockComponentFactory  alloc] initWithAssembly:[ExceptionTestAssembly assembly]];
//    _circularDependenciesFactory = [[TyphoonBlockComponentFactory alloc] initWithAssembly:[CircularDependenciesAssembly assembly]];
//    _singletonsChainFactory = [[TyphoonBlockComponentFactory alloc] initWithAssembly:[SingletonsChainAssembly assembly]];
}

If I comment out the above two assemblies in BlockComponentFactoryTests then it no longer crashes. ..

@jasperblues
Copy link
Member

I'm only getting the crash with Xcode 5

Works fine with Xcode 4.

@jasperblues
Copy link
Member

With Xcode5, it goes into an infinite loop here:

+ (NSString*)keyForAdvisedSEL:(SEL)advisedSEL
{
    LogDebug(@"$$$$$$$$ here we go $$$$");
    NSString* name = NSStringFromSelector(advisedSEL);
    LogDebug(@"Name: %@", name);
    NSString* key = [name stringByReplacingOccurrencesOfString:TYPHOON_BEFORE_ADVICE_PREFIX withString:@""];
    return key;
}

@jasperblues
Copy link
Member

Update . . . On Xcode 5, enters infinite loop here:

+ (NSMutableArray *)resolveStackForKey:(NSString *)key
{
    NSMutableArray *resolveStack = [resolveStackForKey objectForKey:key];
    if (!resolveStack)
    {
        LogDebug(@"No resolve stack for key: %@ . . making one", key);
        resolveStack = [[NSMutableArray alloc] init];
        [resolveStackForKey setObject:resolveStack forKey:key];
    }
    return resolveStack;
}

@jasperblues
Copy link
Member

Ok - fixed on Xcode 5 now. . . resolveStackForKey was somehow nil. . changed to lazy instantiation.

@cesteban
Copy link
Contributor Author

cesteban commented Sep 6, 2013

Sorry, didn't even try to compile with Xcode 5

@jasperblues
Copy link
Member

No problems, it all works well now. . . I suspect it was a subtle difference in the obj-c runtime lib.

Jasper Blues
Landline: +63.2.826.2489
Skype: jasperblues
LinkedIn Twitter

On Sep 6, 2013, at 4:21 PM, César Estébanez Tascón notifications@github.com wrote:

Sorry, didn't even tried to compile with Xcode 5


Reply to this email directly or view it on GitHub.

@jasperblues
Copy link
Member

As it turns out, I think perhaps we should throw an exception if a circular dependency is used in an initializer. . What if the initializer wants to do something with that dependency right away? It will not be in its final state yet?

The reason I suggest throwing an exception is to match the behavior of Spring for Java.

What do you think?

@jasperblues
Copy link
Member

Your clean up of circular deps is still tremendously useful in any case.

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

Successfully merging this pull request may close these issues.

2 participants