appsquickly / typhoon

Powerful dependency injection for Objective-C ✨✨ (https://PILGRIM.PH is the pure Swift successor to Typhoon!!)✨✨
https://pilgrim.ph
Apache License 2.0
2.7k stars 269 forks source link

Block runtime argument not retaining local variables (32-bit architecture only) #493

Closed novixon closed 8 years ago

novixon commented 8 years ago

Hello, guys! I found a problem.

I want to make a factory method with Typhoon. My TyphoonAssembly is OK - I checked it. The problem is though that I get "exc_bad_access" crash when using the client's code below.

MyClass localVariable = [MyClass new];
[concreteAssembly factoryMethodWithBlockRuntimeArgument:^(id data, NSError *error) {
    if (localVariable) {
        //
    }
}];

I found that this crash happens when localVariable is released (e.g. in the end of the method). I thought that it has to be retained by the block. The crash will not appear if I make the localVariable static, for example.

If I don't use some kind of local variables inside block - my code works fine. Do you have any ideas on this problem?

Thanks!

vasilenkoigor commented 8 years ago

Hello @novixon! Can you give me some more details about your implementation a factory method with block?

novixon commented 8 years ago

Hi @spbvasilenko! Here it is. It not the exact production code, but it is pretty identical. As I said before there is no crash and the code works fine If I don't use localVariable inside a runtime-injected block.

typedef void(^ResultBlock)(id data, NSError *error);
- (void)configureWithResultBlock:(ResultBlock)resultBlock;

- (id)factoryMethodWithBlockRuntimeArgument:(ResultBlock)resultBlock {

    return [TyphoonDefinition withClass:[SomeClass class]
                          configuration:^(TyphoonDefinition *definition) {

                              [definition useInitializer:@selector(defaultInitializer)];

                              SEL configSel = @selector(configureWithResultBlock:);
                              [definition injectMethod:configSel
                                            parameters:^(TyphoonMethod *method) {

                                                [method injectParameterWith:resultBlock];
                                            }];
                          }];
}
vasilenkoigor commented 8 years ago

@novixon hmm..it is strange behaviour. I'm written test code like your example and make some few tests, this work fine with local variables. Local variable is retained into block scope.

novixon commented 8 years ago

@spbvasilenko Check it with iPhone 4/4s/5 simulator/device (32-bit architecture). We've found that problem is hidden there.

vasilenkoigor commented 8 years ago

@novixon Yep, Im reproduce this issue on 32-bit architecture. I have noticed interested behaviour...if result block is NSMallocBlock, code work fine without crashes. If block is NSStackBlock, we have a crash.

NSObject *object = [NSObject new];
 __weak typeof(object) weakObject = object;
void (^block)(id data, NSError *error) = ^(id data, NSError *error) {
      __weak typeof(object) strongObject = weakObject;
        if (strongObject) {
            NSLog(@"");
        }
};
[_assembly factoryMethodWithBlockRuntimeArgument:[block copy]];

NSLog(@"%@", object);

This slice example code work without crash.

novixon commented 8 years ago

@spbvasilenko Yep, I got the same result - no crash when defining the result block before passing it as a parameter.

vasilenkoigor commented 8 years ago

@novixon, I'm want to fix this issue and make the MR

novixon commented 8 years ago

@spbvasilenko It's great, thanks! I will research this a little bit on weekends if needed.

alexgarbarev commented 8 years ago

Reviewed. Looks cool! Thanks, guys!