Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
38 changes: 31 additions & 7 deletions ReactiveObjC/NSObject+RACSelectorSignal.m
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,13 @@ static BOOL RACForwardInvocation(id self, NSInvocation *invocation) {

static void RACSwizzleForwardInvocation(Class class) {
SEL forwardInvocationSEL = @selector(forwardInvocation:);
Method forwardInvocationMethod = class_getInstanceMethod(class, forwardInvocationSEL);

// Existing implementation of -forwardInvocation: should be preserved for the preformance.
__block Method preservedForwardInvocationMethod = 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);
__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 @@ -78,10 +79,33 @@ static void RACSwizzleForwardInvocation(Class class) {
BOOL matched = RACForwardInvocation(self, invocation);
if (matched) return;

if (originalForwardInvocation == NULL) {
// 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];
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 (preservedForwardInvocation == NULL) {
[self doesNotRecognizeSelector:invocation.selector];
} else {
originalForwardInvocation(self, forwardInvocationSEL, invocation);
preservedForwardInvocation(self, forwardInvocationSEL, invocation);
}
};

Expand Down
38 changes: 38 additions & 0 deletions ReactiveObjCTests/NSObjectRACSelectorSignalSpec.m
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
@import Quick;
@import Nimble;

#import <objc/message.h>

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

Expand Down Expand Up @@ -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);
Copy link
Member

@andersio andersio Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind to replicate the test case for the case where the intercepted selector by rac_signalForSelector, i.e. lifeIsGood:, is also the one being hot-patched?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I missed it.
The short answer is NO, currently.

Hot-patch implements forwardInvocation: on original class. But the calling process of lifeIsGood: will be intercepted by RACForwardInvocation on dynamic subclass:

BOOL matched = RACForwardInvocation(self, invocation);
if (matched) return;

Good news is, it can be identified by checking (aliasSelector on dynamic subclass != originalSelector on original class) to know whether it has been hot-patched (same situation with method swizzle) or not.

One more step forward, we can just checking (originalSelector on original class == _objc_msgForward) to know it. But I don't recommend it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done.

// 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];

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