swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.61k stars 10.37k forks source link

[SR-7259] Calling protocol method on associated type object causes bad memory access #49807

Open swift-ci opened 6 years ago

swift-ci commented 6 years ago
Previous ID SR-7259
Radar None
Original Reporter Nikola Lajic (JIRA User)
Type Bug

Attachment: Download

Environment macOS High Sierra 10.13.1 (17B1002) Xcode Version 9.2 (9C40b) Reproduced on multiple computers
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 19add1cc56657194f435984ca891b63e

Issue Description:

Calling a method, defined in a protocol, on a property defined in another protocol with an associated type causes an error:

EXC_BAD_ACCESS (code=2, address=0x119cc7b40).

The error will be thrown at the last line of the following code:

protocol HasFactory {
    associatedtype T: AnyObject
    weak var delegate: T? { get set }
}
extension HasFactory where Self: NSObject {
    static func make(with delegate: T) -> Self {
        var instance = self.init()
        instance.delegate = delegate
        return instance
    }
}
protocol IsDelegate: NSObjectProtocol {
    func delegateMethod()
}
class Delegate: NSObject, IsDelegate {
    func delegateMethod() {}
}
class Object: NSObject, HasFactory {
    weak var delegate: IsDelegate?
}
let delegate = Delegate()
let object = Object.make(with: delegate)
object.delegate?.delegateMethod() // error thrown here

Workarounds:

  1. Add @objc to IsDelegate protocol. Which works but is problematic because you have to declare all future delegate protocols as @objc, and there is no warning if you don't, so you risk a runtime crash.

  2. In Object, declare delegate as a class instead of a protocol (in this example Delegate instead of IsDelegate). This avoids the problem but defeats the purpose, since Object is tied to a concrete implementation.

belkadan commented 6 years ago

This is "fixed" in Swift 4.1 by noting that a class-bound protocol doesn't really count as AnyObject, even though it's convertible to it.

<stdin>:20:7: error: type 'Object' does not conform to protocol 'HasFactory'
class Object: NSObject, HasFactory {
      ^
<stdin>:4:20: note: unable to infer associated type 'T' for protocol 'HasFactory'
    associatedtype T: AnyObject
                   ^
<stdin>:21:14: note: inferred type 'IsDelegate' (by matching requirement 'delegate') is invalid: does not conform to 'AnyObject'
    weak var delegate: IsDelegate?
             ^

@slavapestov, do you have the dup for this?

swift-ci commented 6 years ago

Comment by Nikola Lajic (JIRA)

Thanks @belkadan, at least with a warning I won't have to remember to sprinkle @objc. Is there a way to make this work in environments where Objective C is not available?

belkadan commented 6 years ago

:-( Not really. There's a bit more discussion of the issue in SR-6039. You can store the delegate as AnyObject? instead of IsDelegate?, but that requires downcasting when you want to actually use it, which isn't a free operation. There are also some ridiculous tricks you can play with closures and weak captures and protocol extension methods, but I don't know if that's good for maintainability.

protocol HasFactory {
    associatedtype T//: AnyObject
    /*weak*/ var delegate: T? { get set }
}

extension IsDelegate {
  func weaklyCaptureInClosureForSR7259() -> () -> IsDelegate? {
    return { [weak self] in self }
  }
}

class Object2: NSObject, HasFactory {
  private var _delegateStorage: (() -> IsDelegate?)?
  var delegate: IsDelegate? {
    get { return _delegateStorage?() }
    set { _delegateStorage = newValue?.weaklyCaptureInClosureForSR7259() }
  }
}

@slavapestov, anything to add?

swift-ci commented 6 years ago

Comment by Nikola Lajic (JIRA)

Thanks for the workarounds. @objc will work in my case, but hopefully this gets fixed in a future version so it's not environment dependent.

swift-ci commented 6 years ago

Comment by Nikola Lajic (JIRA)

@belkadan searching for an alternative I tried to use generic classes and inheritance.

class A {
    init() {}
}
class B<T: AnyObject>: A {
    weak var delegate: T?
}
protocol Delegate {}
class C: B<Delegate> {}

Which throws an error:

static member 'init()' cannot be used on instance of type 'B<Delegate>'

Adding @objc to Delegate fixes the problem again.

Is this related or should I create a separate issue?

belkadan commented 6 years ago

That's a bad error, but it's just as much not going to work as what you have. Non-@objc protocol values are not compatible with AnyObject-constrained generics. (That's the original SR-6039.)

You could file a separate bug for the error message, though.