Implicit retain cycle


#1

Hi there,

When you call a member method in a closure, the compiler warns you to explicitly capture it.

self.handler = { bar() }  // Call to method 'bar' in closure requires explicit 'self.' to make capture semantics explicit

Then, you write:

self.handler = { self.bar() }

And realize the retain cycle. Therefore, you'll end up writing like:

self.handler = { [unowned self] in self.bar() }

This is good.

However, if you set the method directly because you can, the compiler doesn't give you the warning.

self.handler = bar  // implicit retain cycle here!

I've personally encountered this bug a couple of times in the past (and recently), and I wonder if this would be a common mistake and if so, what the compiler can do to prevent.

Thoughts?


(Robert Widmann) #2

I think it's reasonable to warn about this pattern and suggest a fixit that offers to insert a closure that explicitly captures 'self'. I've filed SR-8536 about this.

I think it makes for a good beginner/intermediate-level starter bug.


(Daniel Resnick) #3

I'd rather just require using self when referring to a method as a value as opposed to needing to wrap the whole thing in a closure.


(Jon Shier) #4

There's no way to break the cycle if you do that. By forcing a closure, adding a [weak self] becomes easy.


(Daniel Resnick) #5

Yeah, I meant using self.method in the case where you explicitly want to capture self strongly. I've often found it nice to be able to directly use a method as a first class function.


(Jacob Williams) #6

Not everyone is trying to avoid the retain cycle because it can have it uses. So a fixit that only suggests wrapping in a closure with weak/unowned self would not be adequate, there should be another fixit to just add the explicit self.

Of course, then how do beginners know which one to pick from? They'll probably go with the easiest fixit of just adding an explicit self. Which is probably ok for beginners anyways since they won't know or probably even care about retain cycles (yet). As they learn more about swift they will eventually learn about weak/unowned references and when/where to use them.


(Tino) #7

Imho it would be quite convenient if you could write self.handler = self?.bar to get a weak capture of self. The weak-strong dance is still one of the biggest annoyances of closures, and it would be nice to avoid some verbosity here.


(Jon Shier) #8

Or even self.handler = [weak self].bar or something.


#9

Hi! I'm totally for adding at least some fixit for this case. I've found that is hard to explain newcomers and even control inside your code such things, because sometimes you want to write shorter and just go with a.callback = myMethod code, but then realize that there could be cycle and change or do not realize and get memory leak, bugs, etc. I see this as an error like "retain cycle possible here, customize capture or implicitly specify self" or any other ways to coupe with this.


(Svein Halvor Halvorsen) #10

In Swift, instance methods are really just curried methods on classes. It might be possible to write a free function that takes a reference to an instance, as well as a function reference to the class method, and returns a closure that captures the supplied instance weakly/unowned and calls the curried class function.

Something like this:

func unown<T: AnyObject, U, V>(_ instance: T, _ classFunction: @escaping (T) -> ((U) -> V)) -> (U) -> V {
    return { [unowned instance] in classFunction(instance)($0) }
}

Or if key paths can point to a method, we could do something like this:

func unown<T: AnyObject, U, V>(_ instance: T, _ pathToClassFunction: KeyPath<T, (U) -> V>) -> (U) -> V {
    return { [unowned instance] in instance[keyPath: pathToClassFunction]($0) }
}

That way, we could write:

self.handler = unown(self, \.bar)

This is probably all just garbage, but it might get some thoughts going? ¯\_(ツ)_/¯


(Amir Abbas Mousavian) #11

Retain cycle is not the reason. Indeed you must explicitly write self in closure because closue is capturing this variable.


(Svein Halvor Halvorsen) #12

Sure, and in by doing so, you might introduce a retain cycle. However, the explicit self makes it (somewhat) easy to spot. However, when you use a function reference as a closure argument, the capture is implicit because the body of that function are free to use self.

As far as I can understand, people want two things:

  1. To make the compiler warn about the implicit capture
  2. To have some syntactic sugar to make it easy to use a function on self as a closure, but without capturing self at all.

Personally, I'd like for Swift to have pure functions on classes, like this: https://gist.github.com/anandabits/02be1c40cdb2a9f90d0c82320b589cda

Functions marked as pure func could be referenced and passed around the same way as free functions, without capturing self


(Kim Burge Strand) #13

I believe they'll look different to the compiler, but there's another case to consider here where bar is already a function property that might (or might not) strongly retain self.

class S {
  let bar: () -> Void
  let handler: () -> Void

  func assign() {
    self.handler = bar // not necessarily a retain cycle, but it _might_ be
  }
}

Additionally, I feel this is similar in spirit to Request explicit capture of self instead of defaulting to a strong capture of self and might be related.


(Amir Abbas Mousavian) #14

I think I've failed to explain. The whole point of that self in closure is not to easily spot retain cycle, but to declare that compiler must capture self for the closure body. This capturing will create a retain cycle.

Technically a closure is a object stored in heap.. Every time you retain a reference to closure, compiler will retain all variables captured by closed, and if you release reference, compiler will release all variables captured by closure.

To find retain cycle you must use Xcode debugging tools or Instruments.


(Jon Shier) #15

No it's not. It's a strictly additive requirement to make implicit self captures visible in code. It doesn't guarantee there's a retain cycle either.


(Svein Halvor Halvorsen) #16

I think I understood that point. And I’m not questioning it. However, regardless of the point, it is de facto the case, that the presence of the self makes it easier to reason about the capture, including possible retain cycles.

When you pass a reference to an instance method as a closure argument, or assign it to a variable, that method is implicitly dependent on its instance, which is thus captured. This is however, not very obvious. Especially if the function is in reality pure, with no real dependence on self.

eventHandler = { handleEvent($0) }

When i do this the compiler asks me to insert self, as you say, to capture it. Even though the function body of handleEvent is strictly pure, this requirement has the effect of reminding me that self is captured. Even if this may not be it’s purpose, it is still the case.

But when I write this:

eventHandler = handleEvent

… I still capture self, albeit much less obviously.


(Simon Seyer) #17

I also ran into this issue and it was really not obvious to me why I got a retain cycle. Obviously it makes sense when you think about it — but as in the closure case the compiler should, in my opinion, make you think about it to avoid issues down the road.

I guess there should be a way to define a capture list in some way when passing functions around.

It would be weird, though, to write the capture list when passing the function because the definition of the capture list is separated from the function definition (same problem I see with the suggestions above, in addition to that they only really work for self I guess) — maybe it could be allowed to define a capture list for a function and warn about the case when functions without a capture list are passed around.


Related question: Can you explain to me, why a strong reference is the default for closures and not the "safer" weak reference? If you would be required to write [strong self] it would be very obvious that you just captured self


(Mox) #18

It would be actually really great, if the default capture of self in closures would be weak one.

So if you don't do any [xxxx self] -style captures in closure, you'd have to write self?.variable when using a variable from self.

That way, the default is the safe one, and you could do [unowned self] or [strong self] as explicit alternatives when you know what you are doing.


(Svein Halvor Halvorsen) #19

Are there any evidence that the weak self pattern is more commonly correct than the strong self? What is most used in the compatibility suite?

I think I want the default strong capture almost always, but I mean guy be mistaken. However, I really don’t want self to have different capture semantics than other variables, and I’m pretty certain I almost never want to weekly capture non-self.

Also, often people capture self, only to access a single property of self for read-only access, or to call some non-mutating function on it, or to access a reference type on self. In all those cases, you can explicitly capture just that single parameter instead, and avoid capturing self altogether. The fixit should offer this solution rather than inserting self.


(Tino) #20

I was thinking about starting a thread to discuss this — but I think it's to late for such a change :-( (and their might be some downsides as well... after all, I guess there is a reason for the status quo).
It might be possible to make a capture weak when you use the variable as an Optional, but that would feel a little bit odd for me, and make the language more complicated.