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

Fix crashes when using Aspects, jsPatch or waxPatch… #27

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 53 additions & 12 deletions ReactiveObjC/NSObject+RACSelectorSignal.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:.
Expand All @@ -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);
}
};

Expand Down
123 changes: 122 additions & 1 deletion ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@
@import Quick;
@import Nimble;

#import <objc/message.h>

#import "RACTestObject.h"
#import "RACSubclassObject.h"

#import <ReactiveObjC/EXTScope.h>

#import "NSObject+RACDeallocating.h"
#import "NSObject+RACPropertySubscribing.h"
#import "NSObject+RACSelectorSignal.h"
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -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];

Expand Down
2 changes: 2 additions & 0 deletions ReactiveObjCTests/RACTestObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions ReactiveObjCTests/RACTestObject.m
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ + (void)lifeIsGood:(id)sender {

}

- (void)methodHotpatch
{

}

- (NSRange)returnRangeValueWithObjectValue:(id)objectValue andIntegerValue:(NSInteger)integerValue {
return NSMakeRange((NSUInteger)[objectValue integerValue], (NSUInteger)integerValue);
}
Expand Down