diff --git a/ReactiveObjC/NSObject+RACSelectorSignal.m b/ReactiveObjC/NSObject+RACSelectorSignal.m index 63e232003..72355b11c 100644 --- a/ReactiveObjC/NSObject+RACSelectorSignal.m +++ b/ReactiveObjC/NSObject+RACSelectorSignal.m @@ -38,15 +38,40 @@ @implementation NSObject (RACSelectorSignal) -static BOOL RACForwardInvocation(id self, NSInvocation *invocation) { +static BOOL RACForwardInvocation(id self, NSInvocation *invocation, void (*forwardInvocation)(id, SEL, NSInvocation *)) { SEL aliasSelector = RACAliasForSelector(invocation.selector); RACSubject *subject = objc_getAssociatedObject(self, aliasSelector); Class class = object_getClass(invocation.target); BOOL respondsToAlias = [class instancesRespondToSelector:aliasSelector]; if (respondsToAlias) { - invocation.selector = aliasSelector; - [invocation invoke]; + BOOL updatedOrForwarded = NO; + + // Fetch latest implementation of aliasSelector from originalClass + Class originalClass = [self class]; + if (originalClass != class) { + + // Detect changing of aliasSelector we saved previously + Method originalInvocationMethod = class_getInstanceMethod(originalClass, invocation.selector); + Method aliasInvocationMethod = class_getInstanceMethod(class, aliasSelector); + void (*originalInvocation)(id, SEL, NSInvocation *) = (__typeof__(originalInvocation))method_getImplementation(originalInvocationMethod); + void (*aliasInvocation)(id, SEL, NSInvocation *) = (__typeof__(aliasInvocation))method_getImplementation(aliasInvocationMethod); + if (originalInvocation != aliasInvocation) { + if ((IMP)originalInvocation == _objc_msgForward) { + // Hotpatch: method re-implemented with `forwardInvocation:` + originalInvocation = forwardInvocation; + } + if (originalInvocation != NULL) { + originalInvocation(self, invocation.selector, invocation); + updatedOrForwarded = YES; + } + } + } + + if (! updatedOrForwarded) { + invocation.selector = aliasSelector; + [invocation invoke]; + } } if (subject == nil) return respondsToAlias; @@ -57,12 +82,12 @@ static BOOL RACForwardInvocation(id self, NSInvocation *invocation) { static void RACSwizzleForwardInvocation(Class class) { SEL forwardInvocationSEL = @selector(forwardInvocation:); - Method forwardInvocationMethod = class_getInstanceMethod(class, forwardInvocationSEL); - - // Preserve any existing implementation of -forwardInvocation:. - void (*originalForwardInvocation)(id, SEL, NSInvocation *) = NULL; - if (forwardInvocationMethod != NULL) { - originalForwardInvocation = (__typeof__(originalForwardInvocation))method_getImplementation(forwardInvocationMethod); + + // Existing implementation of -forwardInvocation: should be preserved for the preformance. + Method preservedForwardInvocationMethod = class_getInstanceMethod(class, forwardInvocationSEL); + __block void (*preservedForwardInvocation)(id, SEL, NSInvocation *) = NULL; + if (preservedForwardInvocationMethod != NULL) { + preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(preservedForwardInvocationMethod); } // Set up a new version of -forwardInvocation:. @@ -75,13 +100,29 @@ static void RACSwizzleForwardInvocation(Class class) { // was no existing implementation, throw an unrecognized selector // exception. id newForwardInvocation = ^(id self, NSInvocation *invocation) { - BOOL matched = RACForwardInvocation(self, invocation); + // Fetch latest implementation of -forwardInvocation:. + // + // The preserved `preservedForwardInvocation` may be replaced in later. + // Continue to use old implementation will lead to an unexpected situration. + // + // This process adds the compatibility with libraries such as Aspects, + // jsPatch or waxPatch whom hook functions with forwardInvocation:. + Class originalClass = [self class]; + Class actualClass = object_getClass(self); + + // IMP shouldn't be retrieved while @selector(class) returns the dynamic subclass + if (originalClass != actualClass) { + Method forwardInvocationMethod = class_getInstanceMethod(originalClass, forwardInvocationSEL); + preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(forwardInvocationMethod); + } + + BOOL matched = RACForwardInvocation(self, invocation, preservedForwardInvocation); if (matched) return; - if (originalForwardInvocation == NULL) { + if (preservedForwardInvocation == NULL) { [self doesNotRecognizeSelector:invocation.selector]; } else { - originalForwardInvocation(self, forwardInvocationSEL, invocation); + preservedForwardInvocation(self, forwardInvocationSEL, invocation); } }; diff --git a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m index 7c4afbaa0..6cf73f6ca 100644 --- a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m +++ b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m @@ -9,9 +9,13 @@ @import Quick; @import Nimble; +#import + #import "RACTestObject.h" #import "RACSubclassObject.h" +#import + #import "NSObject+RACDeallocating.h" #import "NSObject+RACPropertySubscribing.h" #import "NSObject+RACSelectorSignal.h" @@ -193,7 +197,7 @@ - (id)objectValue; expect(@([object respondsToSelector:selector])).to(beTruthy()); }); - + qck_it(@"should properly implement -respondsToSelector: when called on signalForSelector'd receiver that has subsequently been KVO'd", ^{ RACTestObject *object = [[RACTestObject alloc] init]; @@ -306,6 +310,123 @@ - (id)objectValue; }); }); +qck_describe(@"hotpatch with dynamic subclassing", ^{ + __block BOOL patchApplied; + __block RACTestObject * object; + __block Class originalClass; + + qck_beforeEach(^{ + patchApplied = NO; + object = [[RACTestObject alloc] init]; + originalClass = RACTestObject.class; + }); + + qck_it(@"should perform hot-patch successfully on a dynamic subclassed class", ^{ + // A dynamic subclass created by KVO and `signalForSelector:`. + [[RACObserve(object, objectValue) publish] connect]; + [object rac_signalForSelector:@selector(lifeIsGood:)]; + + // Simulator of hotpatch: disable a selector then re-implementing it in forwardInvocation: + SEL selectorPatched = @selector(methodHotpatch); + + // Step 1: disable original selector + Method method = class_getInstanceMethod(originalClass, selectorPatched); + const char *typeDescription = (char *)method_getTypeEncoding(method); + IMP originalImp = class_replaceMethod(originalClass, selectorPatched, _objc_msgForward, typeDescription); + @onExit { // restore hotpatch + class_replaceMethod(originalClass, selectorPatched, originalImp, typeDescription); + }; + + // Step 2: re-implementing it in forwardInvocation: (similar process as jsPatch) + id patchForwardInvocationBlock = ^(id self, NSInvocation *invocation) { + if (invocation.selector == selectorPatched) { + expect(@(patchApplied)).to(beFalsy()); + patchApplied = YES; + } + }; + IMP hpForwardInvocation = imp_implementationWithBlock(patchForwardInvocationBlock); + + // Step 3: applying newly patched selector + IMP originalForwardImp = class_replaceMethod(originalClass, @selector(forwardInvocation:), hpForwardInvocation, "v@:@"); + @onExit { // restore hotpatch + class_replaceMethod(originalClass, @selector(forwardInvocation:), originalForwardImp, "v@:@"); + }; + + // Calling patched method + [object methodHotpatch]; + + expect(@(patchApplied)).to(beTruthy()); + }); + + qck_it(@"should perform hot-patch successfully on a selector intercepted by rac_signalForSelector:", ^{ + // A dynamic subclass created by `signalForSelector:`. + [object rac_signalForSelector:@selector(lifeIsGood:)]; + + // Simulator of hotpatch: disable a selector then re-implementing it in forwardInvocation: + SEL selectorPatched = @selector(lifeIsGood:); + + // Step 1: disable original selector + Method method = class_getInstanceMethod(originalClass, selectorPatched); + const char *typeDescription = (char *)method_getTypeEncoding(method); + IMP originalImp = class_replaceMethod(originalClass, selectorPatched, _objc_msgForward, typeDescription); + @onExit { // restore + class_replaceMethod(originalClass, selectorPatched, originalImp, typeDescription); + }; + + // Step 2: re-implementing it in forwardInvocation: (similar process as jsPatch) + id patchForwardInvocationBlock = ^(id self, NSInvocation *invocation) { + if (invocation.selector == selectorPatched) { + expect(@(patchApplied)).to(beFalsy()); + patchApplied = YES; + } + }; + IMP hpForwardInvocation = imp_implementationWithBlock(patchForwardInvocationBlock); + + // Step 3: applying newly patched selector + IMP originalForwardImp = class_replaceMethod(originalClass, @selector(forwardInvocation:), hpForwardInvocation, "v@:@"); + @onExit { // restore + class_replaceMethod(originalClass, @selector(forwardInvocation:), originalForwardImp, "v@:@"); + }; + + // Calling patched method + [object lifeIsGood:nil]; + + expect(@(patchApplied)).to(beTruthy()); + }); + + qck_it(@"should perform method-swizzling successfully on a selector intercepted by rac_signalForSelector:", ^{ + // A dynamic subclass created by `signalForSelector:`. + [object rac_signalForSelector:@selector(lifeIsGood:)]; + + // Simulator of hotpatch: disable a selector then re-implementing it in forwardInvocation: + SEL selectorPatched = @selector(lifeIsGood:); + + // Step 1: typeDescription on original selector + Method method = class_getInstanceMethod(originalClass, selectorPatched); + const char *typeDescription = (char *)method_getTypeEncoding(method); + + // Step 2: re-implementing method + id methodSwizzlingBlock = ^(id self, NSInvocation *invocation) { + if (invocation.selector == selectorPatched) { + expect(@(patchApplied)).to(beFalsy()); + patchApplied = YES; + } + }; + IMP methodSwizzlingInvocation = imp_implementationWithBlock(methodSwizzlingBlock); + + // Step 3: applying newly patched selector + IMP originalImp = class_replaceMethod(originalClass, selectorPatched, methodSwizzlingInvocation, typeDescription); + @onExit { // restore + class_replaceMethod(originalClass, selectorPatched, originalImp, typeDescription); + }; + + // Calling patched method + [object lifeIsGood:nil]; + + expect(@(patchApplied)).to(beTruthy()); + }); +}); + qck_it(@"should swizzle an NSObject method", ^{ NSObject *object = [[NSObject alloc] init]; diff --git a/ReactiveObjCTests/RACTestObject.h b/ReactiveObjCTests/RACTestObject.h index 98dd7393c..de98711e0 100644 --- a/ReactiveObjCTests/RACTestObject.h +++ b/ReactiveObjCTests/RACTestObject.h @@ -72,6 +72,8 @@ typedef struct { + (void)lifeIsGood:(id)sender; +- (void)methodHotpatch; + - (NSRange)returnRangeValueWithObjectValue:(id)objectValue andIntegerValue:(NSInteger)integerValue; // Writes 5 to the int pointed to by intPointer. diff --git a/ReactiveObjCTests/RACTestObject.m b/ReactiveObjCTests/RACTestObject.m index 66aac010a..c1c9cd4b7 100644 --- a/ReactiveObjCTests/RACTestObject.m +++ b/ReactiveObjCTests/RACTestObject.m @@ -64,6 +64,11 @@ + (void)lifeIsGood:(id)sender { } +- (void)methodHotpatch +{ + +} + - (NSRange)returnRangeValueWithObjectValue:(id)objectValue andIntegerValue:(NSInteger)integerValue { return NSMakeRange((NSUInteger)[objectValue integerValue], (NSUInteger)integerValue); }