Skip to content

Commit

Permalink
Merge pull request #3435 from ReactiveCocoa/swizzling-kvo-interop-fix
Browse files Browse the repository at this point in the history 
Improve the interoperability of all ObjC swizzling activities in the framework.
  • Loading branch information
@mdiep
mdiep committed Apr 8, 2017
2 parents a5817a1 + 58414f7 commit c62a51b
Show file tree
Hide file tree
Showing 8 changed files with 600 additions and 43 deletions.
4 changes: 4 additions & 0 deletions ReactiveCocoa.xcodeproj/project.pbxproj
View file
Expand Up @@ -63,6 +63,7 @@
834DE1121E4120340099F4E5 /* NSSegmentedControl.swift in Sources */ = {isa = PBXBuildFile; fileRef = 834DE1111E4120340099F4E5 /* NSSegmentedControl.swift */; };
834DE1141E4122910099F4E5 /* NSSlider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 834DE1131E4122910099F4E5 /* NSSlider.swift */; };
8392D8FD1DB93E5E00504ED4 /* NSImageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8392D8FC1DB93E5E00504ED4 /* NSImageView.swift */; };
9A0726F31E912B610081F3F7 /* ActionProxySpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A0726F21E912B610081F3F7 /* ActionProxySpec.swift */; };
9A1D05E01D93E99100ACF44C /* NSObject+Association.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A1D05DF1D93E99100ACF44C /* NSObject+Association.swift */; };
9A1D05E11D93E99100ACF44C /* NSObject+Association.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A1D05DF1D93E99100ACF44C /* NSObject+Association.swift */; };
9A1D05E21D93E99100ACF44C /* NSObject+Association.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A1D05DF1D93E99100ACF44C /* NSObject+Association.swift */; };
Expand Down Expand Up @@ -378,6 +379,7 @@
834DE1111E4120340099F4E5 /* NSSegmentedControl.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSSegmentedControl.swift; sourceTree = "<group>"; };
834DE1131E4122910099F4E5 /* NSSlider.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSSlider.swift; sourceTree = "<group>"; };
8392D8FC1DB93E5E00504ED4 /* NSImageView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSImageView.swift; sourceTree = "<group>"; };
9A0726F21E912B610081F3F7 /* ActionProxySpec.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ActionProxySpec.swift; sourceTree = "<group>"; };
9A1D05DF1D93E99100ACF44C /* NSObject+Association.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "NSObject+Association.swift"; sourceTree = "<group>"; };
9A1D05EC1D93E9F100ACF44C /* UIActivityIndicatorView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIActivityIndicatorView.swift; sourceTree = "<group>"; };
9A1D05ED1D93E9F100ACF44C /* UIBarButtonItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIBarButtonItem.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -692,6 +694,7 @@
9ADE4A8C1DA6D94C005C2AC8 /* AppKit */ = {
isa = PBXGroup;
children = (
9A0726F21E912B610081F3F7 /* ActionProxySpec.swift */,
4ABEFE311DCFD05F0066A8C2 /* NSCollectionViewSpec.swift */,
9ADE4A8D1DA6D965005C2AC8 /* NSControlSpec.swift */,
4ABEFE2C1DCFD0180066A8C2 /* NSTableViewSpec.swift */,
Expand Down Expand Up @@ -1362,6 +1365,7 @@
buildActionMask = 2147483647;
files = (
834DE1011E4109750099F4E5 /* NSImageViewSpec.swift in Sources */,
9A0726F31E912B610081F3F7 /* ActionProxySpec.swift in Sources */,
9A6AAA0E1DB6A4CF0013AAEA /* InterceptingSpec.swift in Sources */,
9A54A2111DDF5B4D001739B3 /* InterceptingPerformanceTests.swift in Sources */,
538DCB7D1DCA5E9B00332880 /* NSLayoutConstraintSpec.swift in Sources */,
Expand Down
44 changes: 33 additions & 11 deletions ReactiveCocoa/AppKit/ActionProxy.swift
View file
Expand Up @@ -2,7 +2,7 @@ import AppKit
import ReactiveSwift
import enum Result.NoError

internal final class ActionProxy<Owner: AnyObject> {
internal final class ActionProxy<Owner: AnyObject>: NSObject {
internal weak var target: AnyObject?
internal var action: Selector?
internal let invoked: Signal<Owner, NoError>
Expand All @@ -19,7 +19,11 @@ internal final class ActionProxy<Owner: AnyObject> {
// In AppKit, action messages always have only one parameter.
@objc func invoke(_ sender: Any?) {
if let action = action {
NSApp.sendAction(action, to: target, from: sender)
if let app = NSApp {
app.sendAction(action, to: target, from: sender)
} else {
_ = target?.perform(action, with: sender)
}
}

observer.send(value: owner)
Expand All @@ -42,23 +46,41 @@ extension Reactive where Base: NSObject, Base: ActionMessageSending {
return proxy
}

let proxy = ActionProxy<Base>(owner: base, lifetime: lifetime)
base.associations.setValue(proxy, forKey: key)
let superclass: AnyClass = class_getSuperclass(swizzleClass(base))

let proxy = ActionProxy<Base>(owner: base, lifetime: lifetime)
proxy.target = base.target
proxy.action = base.action

base.target = proxy
base.action = #selector(proxy.invoke(_:))
// Set the proxy as the new delegate with all dynamic interception bypassed
// by directly invoking setters in the original class.
typealias TargetSetter = @convention(c) (NSObject, Selector, AnyObject?) -> Void
typealias ActionSetter = @convention(c) (NSObject, Selector, Selector?) -> Void

let setTargetImpl = class_getMethodImplementation(superclass, #selector(setter: base.target))
unsafeBitCast(setTargetImpl, to: TargetSetter.self)(base, #selector(setter: base.target), proxy)

let setActionImpl = class_getMethodImplementation(superclass, #selector(setter: base.action))
unsafeBitCast(setActionImpl, to: ActionSetter.self)(base, #selector(setter: base.action), #selector(proxy.invoke(_:)))

base.associations.setValue(proxy, forKey: key)

let newTargetSetterImpl: @convention(block) (NSObject, AnyObject?) -> Void = { object, target in
let proxy = object.associations.value(forKey: key)!
proxy.target = target
if let proxy = object.associations.value(forKey: key) {
proxy.target = target
} else {
let impl = class_getMethodImplementation(superclass, #selector(setter: self.base.target))
unsafeBitCast(impl, to: TargetSetter.self)(object, #selector(setter: self.base.target), target)
}
}

let newActionSetterImpl: @convention(block) (NSObject, Selector?) -> Void = { object, selector in
let proxy = object.associations.value(forKey: key)!
proxy.action = selector
let newActionSetterImpl: @convention(block) (NSObject, Selector?) -> Void = { object, action in
if let proxy = object.associations.value(forKey: key) {
proxy.action = action
} else {
let impl = class_getMethodImplementation(superclass, #selector(setter: self.base.action))
unsafeBitCast(impl, to: ActionSetter.self)(object, #selector(setter: self.base.action), action)
}
}

// Swizzle the instance only after setting up the proxy.
Expand Down
47 changes: 27 additions & 20 deletions ReactiveCocoa/DelegateProxy.swift
View file
Expand Up @@ -53,57 +53,64 @@ extension DelegateProxy {
internal static func proxy<P: DelegateProxy<Delegate>>(
for instance: NSObject,
setter: Selector,
getter: Selector,
_ key: StaticString = #function
getter: Selector
) -> P {
return _proxy(for: instance, setter: setter, getter: getter, key) as! P
return _proxy(for: instance, setter: setter, getter: getter) as! P
}

private static func _proxy(
for instance: NSObject,
setter: Selector,
getter: Selector,
_ key: StaticString = #function
getter: Selector
) -> AnyObject {
let key = AssociationKey<DelegateProxy<Delegate>?>(key)

return instance.synchronized {
let key = AssociationKey<AnyObject?>(setter.delegateProxyAlias)

if let proxy = instance.associations.value(forKey: key) {
return proxy
}

let superclass: AnyClass = class_getSuperclass(swizzleClass(instance))

let invokeSuperSetter: @convention(c) (NSObject, AnyClass, Selector, AnyObject?) -> Void = { object, superclass, selector, delegate in
typealias Setter = @convention(c) (NSObject, Selector, AnyObject?) -> Void
let impl = class_getMethodImplementation(superclass, selector)
unsafeBitCast(impl, to: Setter.self)(object, selector, delegate)
}

let newSetterImpl: @convention(block) (NSObject, AnyObject?) -> Void = { object, delegate in
let proxy = object.associations.value(forKey: key)!
proxy.forwardee = (delegate as! Delegate?)
if let proxy = object.associations.value(forKey: key) as! DelegateProxy<Delegate>? {
proxy.forwardee = (delegate as! Delegate?)
} else {
invokeSuperSetter(object, superclass, setter, delegate)
}
}

// Hide the original setter, and redirect subsequent delegate assignment
// to the proxy.
instance.swizzle((setter, newSetterImpl), key: hasSwizzledKey)

// Set the proxy as the delegate.
let realClass: AnyClass = class_getSuperclass(object_getClass(instance))
let originalSetterImpl: IMP = class_getMethodImplementation(realClass, setter)
let getterImpl: IMP = class_getMethodImplementation(realClass, getter)

typealias Setter = @convention(c) (NSObject, Selector, AnyObject?) -> Void
typealias Getter = @convention(c) (NSObject, Selector) -> AnyObject?

// As Objective-C classes may cache the information of their delegate at
// the time the delegates are set, the information has to be "flushed"
// whenever the proxy forwardee is replaced or a selector is intercepted.
let proxy = self.init(lifetime: instance.reactive.lifetime) { [weak instance] proxy in
guard let instance = instance else { return }
unsafeBitCast(originalSetterImpl, to: Setter.self)(instance, setter, proxy)
invokeSuperSetter(instance, superclass, setter, proxy)
}

instance.associations.setValue(proxy, forKey: key)
typealias Getter = @convention(c) (NSObject, Selector) -> AnyObject?
let getterImpl: IMP = class_getMethodImplementation(object_getClass(instance), getter)
let original = unsafeBitCast(getterImpl, to: Getter.self)(instance, getter) as! Delegate?

// `proxy.forwardee` would invoke the original setter regardless of
// `original` being `nil` or not.
let original = unsafeBitCast(getterImpl, to: Getter.self)(instance, getter) as! Delegate?
proxy.forwardee = original

// The proxy must be associated after it is set as the target, since
// `base` may be an isa-swizzled instance that is using the injected
// setters above.
instance.associations.setValue(proxy, forKey: key)

return proxy
}
}
Expand Down
42 changes: 36 additions & 6 deletions ReactiveCocoa/NSObject+Intercepting.swift
View file
Expand Up @@ -117,7 +117,8 @@ extension NSObject {
.flatMap { $0 != _rac_objc_msgForward ? $0 : nil }

if let impl = immediateImpl {
class_addMethod(subclass, interopAlias, impl, typeEncoding)
let succeeds = class_addMethod(subclass, interopAlias, impl, typeEncoding)
precondition(succeeds, "RAC attempts to swizzle a selector that has message forwarding enabled with a runtime injected implementation. This is unsupported in the current version.")
}
}
}
Expand All @@ -127,7 +128,7 @@ extension NSObject {

// Start forwarding the messages of the selector.
_ = class_replaceMethod(subclass, selector, _rac_objc_msgForward, typeEncoding)

return state.signal
}
}
Expand Down Expand Up @@ -166,10 +167,39 @@ private func enableMessageForwarding(_ realClass: AnyClass, _ selectorCache: Sel
//
// However, the IMP cache would be thrashed due to the swapping.

let interopImpl = class_getMethodImplementation(realClass, interopAlias)
let previousImpl = class_replaceMethod(realClass, selector, interopImpl, typeEncoding)
invocation.invoke()
_ = class_replaceMethod(realClass, selector, previousImpl, typeEncoding)
let topLevelClass: AnyClass = object_getClass(objectRef.takeUnretainedValue())

// The locking below prevents RAC swizzling attempts from intervening the
// invocation.
//
// Given the implementation of `swizzleClass`, `topLevelClass` can only be:
// (1) the same as `realClass`; or (2) a subclass of `realClass`. In other
// words, this would deadlock only if the locking order is not followed in
// other nested locking scenarios of these metaclasses at compile time.

synchronized(topLevelClass) {
func swizzle() {
let interopImpl = class_getMethodImplementation(topLevelClass, interopAlias)

let previousImpl = class_replaceMethod(topLevelClass, selector, interopImpl, typeEncoding)
invocation.invoke()

_ = class_replaceMethod(topLevelClass, selector, previousImpl, typeEncoding)
}

if topLevelClass != realClass {
synchronized(realClass) {
// In addition to swapping in the implementation, the message
// forwarding needs to be temporarily disabled to prevent circular
// invocation.
_ = class_replaceMethod(realClass, selector, nil, typeEncoding)
swizzle()
_ = class_replaceMethod(realClass, selector, _rac_objc_msgForward, typeEncoding)
}
} else {
swizzle()
}
}

return
}
Expand Down
8 changes: 7 additions & 1 deletion ReactiveCocoa/ObjC+RuntimeSubclassing.swift
View file
Expand Up @@ -33,7 +33,13 @@ extension NSObject {
let method = class_getInstanceMethod(subclass, selector)
let typeEncoding = method_getTypeEncoding(method)!

class_replaceMethod(subclass, selector, imp_implementationWithBlock(body), typeEncoding)
if method_getImplementation(method) == _rac_objc_msgForward {
let succeeds = class_addMethod(subclass, selector.interopAlias, imp_implementationWithBlock(body), typeEncoding)
precondition(succeeds, "RAC attempts to swizzle a selector that has message forwarding enabled with a runtime injected implementation. This is unsupported in the current version.")
} else {
let succeeds = class_addMethod(subclass, selector, imp_implementationWithBlock(body), typeEncoding)
precondition(succeeds, "RAC attempts to swizzle a selector that has already a runtime injected implementation. This is unsupported in the current version.")
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions ReactiveCocoa/ObjC+Selector.swift
View file
Expand Up @@ -17,6 +17,11 @@ extension Selector {
return prefixing("rac1_")
}

/// An alias of `self`, used for delegate proxies.
internal var delegateProxyAlias: Selector {
return prefixing("rac2_")
}

internal func prefixing(_ prefix: StaticString) -> Selector {
let length = Int(strlen(utf8Start))
let prefixedLength = length + prefix.utf8CodeUnitCount
Expand Down

0 comments on commit c62a51b

Please sign in to comment.