Implicit retain cycle

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

6 Likes

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.

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? ¯\_(ツ)_/¯

1 Like

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

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

1 Like

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.

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.

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.

1 Like

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.

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

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.

1 Like

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.

2 Likes

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.

Even if changing to weak wouldn’t be possible, would it make sense to improve the fix-it’s on this? It doesn’t feel right that default fixit only adds self, and new developer suddenly has hidden retain cycle in his/her code...

It makes self explicit to increase the visibility of the possible cycle, it doesn’t hide it.

Yup.

self.handler = { bar() }

can be currently fixed to

self.handler = { self.bar() }

while there's a couple of ways to cut the cycle (e.g [weak self]).

For non closure case, there's no such syntax to cut the cycle right now, and maybe we should come up with new syntax for it (e.g. self.handler = [weak self].bar). But even without it, if it can warn and fix-it by inserting self., I think that'd be really helpful.

What I mean is that a beginner programmer doesn’t necessarily know what retain cycles are. So adding ”self.” does indicate that for them. It’s just a way to make a compiler error go away. ”self?.” Is much more explicit indication that one has to pay attention to the lifecycle of the self.

I agree completely, this is by design. Swift has been designed with progressive disclosure in mind. Meaning that beginners won't learn about things like retain cycles until later (whenever that actually is really depends on the developer).

This is not necessarily true though. Let's think about this through a beginners eyes:

If I didn't even know about retain cycles, then a fixit telling me to use an optional of self will make no sense. If I've never seen (or at least understood) strong/weak/unowned references, I would not understand why/how self could be optional. I might just go ahead and add the fixit to get my code compiling, but then I could easily run into undefined behavior when my self object is nil (for whatever reason) and my callback isn't ran because of how optional chaining works. I would have no idea why my code isn't working the way I'd expect it to work.

I do agree that there needs to be a way to be explicit (I like the [weak self].bar idea), but just making developers use an optional version of self will probably confuse beginner developers more than it will help them. For now, there should probably be a fix it to just add an explicit self, exactly like what you'd expect if you'd written your callback inside a closure. In the future, once the beginner has gained some experience and learned a bit about retain cycles, then they will realize what is actually occurring behind the scenes and can remove unwanted retain cycles.

1 Like

Even for developers that know about retain cycle, adding self here does not in any way indicate a retain cycle.

The same code with a ref to a class instead of a method reference does not create retain cycle.

self.foo = self.bar

If foo and bar are not mehods, there is not retain cycle.

I'd like to emphasize, once again, that there is no such thing as a strong or weak reference. There are only strong or weak variables or properties. That is, strongness or weakness is a property of the box that contains the reference, not of the reference itself.

This seductive fallacy tends to infect discussions like this, because it's almost irresistible as shorthand, even amongst experienced developers.

In this case, the self?.bar syntax looks like it would promote the fallacy amongst future, less experienced developers, because it looks like a style of referencing, rather than what it would really indicate — weakly capturing self inside the method bar.

I'm not sure there's a good syntax for this, but if you want something vaguely like what you've been proposing, then maybe x = [weak self] in bar would not be inconsistent with closure syntax, or perhaps even x = bar[weak self].

But it's not an easy problem, because unmodified instance method name foo isn't really the closure you'd like it to be.