From 7165ae3a0b0ec4bc74eefd244b5499eeed956798 Mon Sep 17 00:00:00 2001 From: doggy Date: Sat, 5 Nov 2016 10:00:47 +0800 Subject: [PATCH 1/6] Adds the compatibility with libraries such as Aspects, jsPatch or waxPatch whom hook functions with forwardInvocation:. --- ReactiveObjC/NSObject+RACSelectorSignal.m | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/ReactiveObjC/NSObject+RACSelectorSignal.m b/ReactiveObjC/NSObject+RACSelectorSignal.m index 63e232003..cd021c45a 100644 --- a/ReactiveObjC/NSObject+RACSelectorSignal.m +++ b/ReactiveObjC/NSObject+RACSelectorSignal.m @@ -57,13 +57,6 @@ 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); - } // Set up a new version of -forwardInvocation:. // @@ -78,6 +71,20 @@ static void RACSwizzleForwardInvocation(Class class) { BOOL matched = RACForwardInvocation(self, invocation); if (matched) return; + // Fetch newest implementation of -forwardInvocation:. + // + // Implementation we preserved 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]; + Method forwardInvocationMethod = class_getInstanceMethod(originalClass, forwardInvocationSEL); + void (*originalForwardInvocation)(id, SEL, NSInvocation *) = NULL; + if (forwardInvocationMethod != NULL) { + originalForwardInvocation = (__typeof__(originalForwardInvocation))method_getImplementation(forwardInvocationMethod); + } + if (originalForwardInvocation == NULL) { [self doesNotRecognizeSelector:invocation.selector]; } else { From b78960204ac1c84b42ea63684c4cf79fd74f4c7f Mon Sep 17 00:00:00 2001 From: doggy Date: Sat, 5 Nov 2016 22:29:15 +0800 Subject: [PATCH 2/6] [RACSelectorSignal] Test case for xxPatch compatibility. --- .../NSObjectRACSelectorSignalSpec.m | 38 +++++++++++++++++++ ReactiveObjCTests/RACTestObject.h | 2 + ReactiveObjCTests/RACTestObject.m | 5 +++ 3 files changed, 45 insertions(+) diff --git a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m index 7c4afbaa0..bd7558f1b 100644 --- a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m +++ b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m @@ -9,6 +9,8 @@ @import Quick; @import Nimble; +#import + #import "RACTestObject.h" #import "RACSubclassObject.h" @@ -194,6 +196,42 @@ - (id)objectValue; expect(@([object respondsToSelector:selector])).to(beTruthy()); }); + qck_it(@"hotpatch with dynamic subclass: disable a selector then re-implementing it in forwardInvocation:", ^{ + __block BOOL patchApplied = NO; + + RACTestObject *object = [[RACTestObject alloc] init]; + + // A dynamic subclass created. + [[RACObserve(object, objectValue) publish] connect]; + [object rac_signalForSelector:@selector(lifeIsGood:)]; + + // Simulator of hotpatch: + SEL selectorPatched = @selector(methodHotpatch); + // Step 1: get class + Class originalClass = NSClassFromString(@"RACTestObject"); + // Step 2: 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); + // Step 4: re-implementing it in forwardInvocation: (same process as jsPatch) + id patchForwardInvocationBlock = ^(id self, NSInvocation *invocation) { + expect(@(patchApplied)).to(beFalsy()); + patchApplied = YES; + }; + IMP hpForwardInvocation = imp_implementationWithBlock(patchForwardInvocationBlock); + // Step 5: applying newly patched selector + IMP originalForwardImp = class_replaceMethod(originalClass, @selector(forwardInvocation:), hpForwardInvocation, "v@:@"); + + // Calling patched method + [object methodHotpatch]; + + expect(@(patchApplied)).to(beTruthy()); + + // Finish: restore hotpatch + class_replaceMethod(originalClass, selectorPatched, originalImp, typeDescription); + class_replaceMethod(originalClass, @selector(forwardInvocation:), originalForwardImp, "v@:@"); + }); + qck_it(@"should properly implement -respondsToSelector: when called on signalForSelector'd receiver that has subsequently been KVO'd", ^{ RACTestObject *object = [[RACTestObject 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); } From 93df7e1b1db5eb3d166a942963a6c7f595608f74 Mon Sep 17 00:00:00 2001 From: doggy Date: Sat, 5 Nov 2016 22:35:13 +0800 Subject: [PATCH 3/6] [RACSelectorSignal] Improvement: update preserved forwardInvocation: function only if successfully fetched the original class --- ReactiveObjC/NSObject+RACSelectorSignal.m | 29 ++++++++++++++++++----- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/ReactiveObjC/NSObject+RACSelectorSignal.m b/ReactiveObjC/NSObject+RACSelectorSignal.m index cd021c45a..d6e561ec7 100644 --- a/ReactiveObjC/NSObject+RACSelectorSignal.m +++ b/ReactiveObjC/NSObject+RACSelectorSignal.m @@ -57,6 +57,14 @@ static BOOL RACForwardInvocation(id self, NSInvocation *invocation) { static void RACSwizzleForwardInvocation(Class class) { SEL forwardInvocationSEL = @selector(forwardInvocation:); + + // Existing implementation of -forwardInvocation: should be preserved for the preformance. + __block 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:. // @@ -79,16 +87,25 @@ static void RACSwizzleForwardInvocation(Class class) { // This process adds the compatibility with libraries such as Aspects, // jsPatch or waxPatch whom hook functions with forwardInvocation:. Class originalClass = [self class]; - Method forwardInvocationMethod = class_getInstanceMethod(originalClass, forwardInvocationSEL); - void (*originalForwardInvocation)(id, SEL, NSInvocation *) = NULL; - if (forwardInvocationMethod != NULL) { - originalForwardInvocation = (__typeof__(originalForwardInvocation))method_getImplementation(forwardInvocationMethod); + 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); + if (preservedForwardInvocationMethod != forwardInvocationMethod) { + preservedForwardInvocationMethod = forwardInvocationMethod; + + if (preservedForwardInvocationMethod != NULL) { + preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(preservedForwardInvocationMethod); + } else { + preservedForwardInvocation = NULL; + } + } } - if (originalForwardInvocation == NULL) { + if (preservedForwardInvocation == NULL) { [self doesNotRecognizeSelector:invocation.selector]; } else { - originalForwardInvocation(self, forwardInvocationSEL, invocation); + preservedForwardInvocation(self, forwardInvocationSEL, invocation); } }; From 27b5d9f22fe774c03d937542b1bf18c335274c32 Mon Sep 17 00:00:00 2001 From: doggy Date: Wed, 9 Nov 2016 14:00:03 +0800 Subject: [PATCH 4/6] [RACSelectorSignal] Support hot-patching on a selector intercepted by `rac_signalForSelector:` --- ReactiveObjC/NSObject+RACSelectorSignal.m | 55 +++++++++++++++-------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/ReactiveObjC/NSObject+RACSelectorSignal.m b/ReactiveObjC/NSObject+RACSelectorSignal.m index d6e561ec7..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; @@ -59,8 +84,7 @@ static void RACSwizzleForwardInvocation(Class class) { SEL forwardInvocationSEL = @selector(forwardInvocation:); // Existing implementation of -forwardInvocation: should be preserved for the preformance. - __block Method preservedForwardInvocationMethod = class_getInstanceMethod(class, forwardInvocationSEL); - + Method preservedForwardInvocationMethod = class_getInstanceMethod(class, forwardInvocationSEL); __block void (*preservedForwardInvocation)(id, SEL, NSInvocation *) = NULL; if (preservedForwardInvocationMethod != NULL) { preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(preservedForwardInvocationMethod); @@ -76,32 +100,25 @@ 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); - if (matched) return; - - // Fetch newest implementation of -forwardInvocation:. + // Fetch latest implementation of -forwardInvocation:. // - // Implementation we preserved may be replaced in later. + // 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); - if (preservedForwardInvocationMethod != forwardInvocationMethod) { - preservedForwardInvocationMethod = forwardInvocationMethod; - - if (preservedForwardInvocationMethod != NULL) { - preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(preservedForwardInvocationMethod); - } else { - preservedForwardInvocation = NULL; - } - } + preservedForwardInvocation = (__typeof__(preservedForwardInvocation))method_getImplementation(forwardInvocationMethod); } + BOOL matched = RACForwardInvocation(self, invocation, preservedForwardInvocation); + if (matched) return; + if (preservedForwardInvocation == NULL) { [self doesNotRecognizeSelector:invocation.selector]; } else { From dafc82d3abb719c2347b5846e27f048f3ad5f500 Mon Sep 17 00:00:00 2001 From: doggy Date: Wed, 9 Nov 2016 14:02:30 +0800 Subject: [PATCH 5/6] [RACSelectorSignal] New test cases added for IMP swizzling and hot-patch --- .../NSObjectRACSelectorSignalSpec.m | 157 +++++++++++++----- 1 file changed, 120 insertions(+), 37 deletions(-) diff --git a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m index bd7558f1b..115afa6ca 100644 --- a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m +++ b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m @@ -14,6 +14,8 @@ #import "RACTestObject.h" #import "RACSubclassObject.h" +#import + #import "NSObject+RACDeallocating.h" #import "NSObject+RACPropertySubscribing.h" #import "NSObject+RACSelectorSignal.h" @@ -195,43 +197,7 @@ - (id)objectValue; expect(@([object respondsToSelector:selector])).to(beTruthy()); }); - - qck_it(@"hotpatch with dynamic subclass: disable a selector then re-implementing it in forwardInvocation:", ^{ - __block BOOL patchApplied = NO; - - RACTestObject *object = [[RACTestObject alloc] init]; - - // A dynamic subclass created. - [[RACObserve(object, objectValue) publish] connect]; - [object rac_signalForSelector:@selector(lifeIsGood:)]; - - // Simulator of hotpatch: - SEL selectorPatched = @selector(methodHotpatch); - // Step 1: get class - Class originalClass = NSClassFromString(@"RACTestObject"); - // Step 2: 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); - // Step 4: re-implementing it in forwardInvocation: (same process as jsPatch) - id patchForwardInvocationBlock = ^(id self, NSInvocation *invocation) { - expect(@(patchApplied)).to(beFalsy()); - patchApplied = YES; - }; - IMP hpForwardInvocation = imp_implementationWithBlock(patchForwardInvocationBlock); - // Step 5: applying newly patched selector - IMP originalForwardImp = class_replaceMethod(originalClass, @selector(forwardInvocation:), hpForwardInvocation, "v@:@"); - - // Calling patched method - [object methodHotpatch]; - - expect(@(patchApplied)).to(beTruthy()); - - // Finish: restore hotpatch - class_replaceMethod(originalClass, selectorPatched, originalImp, typeDescription); - class_replaceMethod(originalClass, @selector(forwardInvocation:), originalForwardImp, "v@:@"); - }); - + qck_it(@"should properly implement -respondsToSelector: when called on signalForSelector'd receiver that has subsequently been KVO'd", ^{ RACTestObject *object = [[RACTestObject alloc] init]; @@ -344,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]; From 6cfa5114986e54c126c61ca638a31bdb01682df9 Mon Sep 17 00:00:00 2001 From: doggy Date: Thu, 10 Nov 2016 09:03:12 +0800 Subject: [PATCH 6/6] [RACSelectorSignal] dummy commit --- ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m index 115afa6ca..6cf73f6ca 100644 --- a/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m +++ b/ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m @@ -393,7 +393,7 @@ - (id)objectValue; 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:)];