[SR-14934] Self can't be assigned weakly within an NSObject's deinit.

[swift-ci]

Previous ID	SR-14934
Radar	rdar://problem/80820401
Original Reporter	hippie_hippo (JIRA User)
Type	Bug

Environment

Xcode 13.0 beta 3 (13A5192i)

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Bug, Foundation
Assignee	None
Priority	Medium

md5: a4dbd7e0645831e526fb5f08d85de61d

Issue Description:

Self can't be assigned weakly within an NSObject's deinit.

Test case:

import Foundation
protocol FooDelegate: AnyObject {}
class Foo {
  weak var delegate: FooDelegate?
  init(delegate: FooDelegate) {
  self.delegate = delegate
 }
 func cleanup() {}
}
class Bar: NSObject, FooDelegate {
  lazy var foo = Foo(delegate: self)
  deinit {
    self.foo.cleanup()
  }
}
var b: Bar? = Bar()
b = nil

Results in

objc[66436]: Cannot form weak reference to instance (0x600000494070) of class __lldb_expr_25.Bar. It is possible that this object was over-released, or is in the process of deallocation.

Note that this does not occur if self is a plain Swift class.

This is not uncommon in UIViewController code, and can result in hard-to-debug issues.

If this isn't a fixable issue, then I'll post a feature request about better handling of lazy vars in deinit.

[typesanitizer]

@swift-ci create

[mikeash]

It's a deliberate choice for ObjC weak references to fatal error when attempting to place a deallocating object into one. Swift's own weak references instead choose to safely store nil. Both are valid, but the inconsistency is annoying. This certainly could be changed, but I'm not sure if it's the right thing to do.

For a case like this, you probably don't want lazy anyway. In the crashing case, when lazy var hasn't been accessed yet, this code runs the initializer and then throws the value away, wasting a bunch of effort. You could instead implement lazy semantics manually, which would allow you to skip the whole thing in deinit if the value was never set. It might be worth somehow supporting that for the built-in lazy feature as well.

[swift-ci]

Comment by Jason Persampieri (JIRA)

Agreed that in this case you wouldn't want lazy, but in reality this is non-trivial to catch. And I don't want to wholesale prohibit lazy, of course. Internally I've started recommending that we always be on the lookout for lazy var accesses in deinit. But that's a half-measure.

If we did want to formalize something, I wonder if we could add Optional-like access to all lazy vars. That is, in addition to

self.lazyVar.cleanup()

that is guaranteed to create lazyVar if necessary, we also offer

self.lazyVar?.cleanup()

that would call cleanup only if lazyVar is already created?

It's still up the developer to notice, so it's not perfect, but I would like to think it would lead to more attention being paid. And if changing the ObjC behavior isn't an option, it's probably the best we can hope for.

But that's more a proposal than a bug fix, I guess 🙂

[mikeash]

I like that idea a lot. I'd take it further and ban the non-Optional version in deinit once the potential source breakage allows for it.

[swift-ci]

Comment by Jason Persampieri (JIRA)

On further thought, that might not work since the lazy var could actually be Optional. Hmm... How could we make that work?

[swift-ci]

Comment by Jason Persampieri (JIRA)

My teammates have suggested introducing the interrobang operator for this.

self.lazyVar‽.cleanup()

I obviously need new teammates.

[mikeash]

I mean, it's kind of hard to type. How about self.lazyVar¿.cleanup()? On a standard Mac keyboard, you can get that with option-shift-?, nice and easy.

Taking this seriously for a moment, something with $ like we do for property wrappers might be the way to go. lazy probably ought to just be a property wrapper at this point.

[parkera]

This should be reopened in the Swift repository.