[swift-evolution] [Draft] @selfsafe: a new way to avoid reference cycles

[Matthew Johnson]

Sent from my iPad

On Feb 19, 2017, at 6:30 PM, Brent Royal-Gordon <brent at architechies.com> wrote:

>> On Feb 19, 2017, at 6:45 AM, Matthew Johnson <matthew at anandabits.com> wrote:
>> 
>> 1. Swift *already* acknowledges that it is far easier to create a reference cycle through captured strong references to `self` than any other way.  This is why you have to explicitly say `self.` in escaping closures.
> 
> The reason you have to explicitly say `self` is that `self` is the only variable you can implicitly reference. That's disabled in closures, so it's on the same footing as other variables.

It's only disabled in escaping closures, not all closures.  It is disabled there specifically because capturing it can produce a reference cycle.  But I agree, while it is by far the most frequent source of cycles it is not the only way to create them and a more general approach is better.

> 
> Other than that, the only reason `self` is more likely to cause a loop is that, in common Cocoa patterns, you're more likely to be handing the closure to something owned by `self`. But that's not *necessarily* the case, it just happens to be true in many cases.

Often you hand it to something owned by self, but it's also the case that you often hand it to something not owned by self, but that should not extend the lifetime of self.  In that case you don't necessarily create a cycle but you do create what is effectively a leak.  In both cases getting a callback after self should have been released can lead to very bad things happening.

The reason self is somewhat unique is that usually when you pass a callback to something the implementation of the callback either modified instance state or calls methods on instance properties.  Occasionally you also capture an argument to the function that registers the callback or connect a method on some other object, but those are far less frequent than starting with your own instance state.  The majority of the time you do need to capture self and much of the time you don't need to capture any other references.  This is (IMO) why self is somewhat special and why API designs like target / action (which only allow "capture" of a single object) work very well.

> 
>> 2. There are *already* APIs which are designed to capture an object weak and then call a method on it if it's still around when an event occurs.  In fact this has been a trend in Apple's newer APIs.  Users are able to learn the semantics of these APIs without a problem.  In fact, users like the, because they solve a real problem by ensuring that3.  object lifetime is not extended when using them.
>> 
>> 3. Swift libraries don't tend to design APIs with weak callback semantics, probably because they are currently syntactically heavy for both users and libraries.  This is true even when weak callback semantics would be beneficial for users and help prevent leaks (which can lead to crashes and other badly behaved apps).
>> 
>> 4. There have been several ideas proposed to make weak capture easier to do on the call side but they haven't gone anywhere.  The syntactic savings aren't that significant for callers and the burden is still on callers to get it right.
> 
> I definitely agree this is a problem, but again, that doesn't mean `self` is the issue here. I think this is mainly because (a) the pattern isn't taught, and (b) there are some minor speed bumps in current Swift (like the inability to shadow `self` in a closure parameter list).

I agree that self isn't the problem.  The biggest problem is that callback APIs that feel Swifty (i.e. just take a function) place the burden of not extending the lifetime of object references on the callee.  It is pretty rare to actually want a callback API to extend the lifetime of an object that you need to use when you are callback.  

In other words, for this class of API there is a significant limitation in the language that makes correct usage much more difficult than it should be.

I feel pretty good about the design I came up with for guarded closures.  This allows us to flip the default capture in a closure from strong to guarded (a new kind of capture) by using the `?` sigil (while still allowing explicit strong capture in the capture list).  APIs are allowed to use the `@guarded` argument annotation to require users to provide a guarded closure.  This means that they must be used with defaults that make sense for their use case while still giving users an easy way to change those defaults if necessary.

> 
>> If users actually need a strong reference there are ways to handle that.  First, if it is likely that a user might want self to be captured strong the API might choose to not use `@selfsafe`.
> 
> This doesn't work. The API designer doesn't know what's at the call site, and the user can't modify the API to suit the use.
> 
>> If they *do* choose to use it the could also add an overload that does not use it and is disambiguated using some other means (base name or argument label).
> 
> That seems like an ugly way to design APIs.

I agree.  It was a sign that I did not have the right design yet.

> 
>> Finally, users can always add an extra closure wrapper {{ self.myMethod() }} to bypass the API's weak callback semantics.  Here, `self` is captured by the inner closure rather than the the outer closure and the API only converts strong `self` references in the outer closure.
> 
> That wouldn't work. The outer closure captures `self` from the context, and the inner closure captures `self` from the outer closure. The outer closure's capture would still be weak.
> 
> I really think this focus on `self` is counterproductive. I'm thinking we might be able to address this in a different way.

I agree.  I'm interested in your thoughts on my second stab at this when you have a chance.  I started a new thread for it called "guarded closures".

> 
> Let's look at your code sample:
> 
>    addAction(takesInt)
> 
> With the implicit `self`s made explicit, that would instead be:
> 
>    self.addAction(
>        self.takesInt
>    )

No, `addAction` was supposed to be a top level function in a library that was called from user code.  I wrote it as a top level function to try and keep the example concise but I see how it was misleading because of that.

> 
> Now let's look at mine:
> 
>    alert.addAction(UIAlertAction(title: "Delete", style: .destructive) { _ in
>        …
>        viewController.performSegue(withIdentifier: "Cancel", sender: self)
>    })
>    …
>        
>    viewController.present(alert)
> 
> These have a similar pattern: The closure ends up being passed to a method on one of the variables it captures. Your example:
> 
>    self.addAction(
>    ^ Closure gets passed to method on `self`
>        self.takesInt
>        ^ Closure captures `self`
>    )
> 
> Mine:
> 
>    alert.addAction(UIAlertAction(title: "Delete", style: .destructive) { _ in
>    ^ Closure passed to method on `alert`
>        …
>        viewController.performSegue(withIdentifier: "Cancel", sender: self)
>        ^ Closure captures `viewController`
>    })
>    …
>        
>    viewController.present(alert)
>    ^ `alert` passed to `viewController`
> 
> This seems like something the compiler—or perhaps a static analyzer?—could warn about. And it wouldn't target `self` specifically, or silently change the meaning of your code.
> 
> -- 
> Brent Royal-Gordon
> Architechies
>