Pitch: Syntactic sugar for circumventing closure capture lists

Sometimes you want to keep the object alive until the callback happens.

For example if you have an object that takes significant time to serialize & save you might want to dispatch_async that to to some background queue and serialize and store it. I would expect that most dispatch_async uses actually want to extend the lifetime of whatever they capture.

Objects participating in some sort of network RPC might want to live long enough for the existing set of outstanding calls to complete (or timeout) and handle the replies. I wouldn't say "most" of these want to extend the lifetimes of any captured objects, but a lot do...or at least frequently wouldn't cause problems.

Sadly we don't have a way to annotate function arguments as "will escape, but has a short lifetime", nor do we even really have a good definition of "short". Does a 0.1 second timer count as "short"? Does the same timer count as short if the target queue might be suspended? Is "when the RPC completes" soon?

That said if weak were the default you will get reminded as soon as you get an "need to unwrap" reminder that you ought to either unwrap the value, or change the capture list to capture self strongly.

1 Like

There’s no option that doesn’t require thinking.

7 Likes

I mean...there kind of is.

thing.doThingWithClosure

return, return

thing.doThingWithClosure { 

}

With the default as [strong self] you can create a reference cycle with zero indication of that possibility, and have a problem that's pretty difficult to figure out without previous knowledge or instruments. You just have to know that's how it works.

If the default were [weak self], you have an indication that there's something different going on, and when self is nil, you have something clear to search for to fix the issue.

1 Like

There are certainly use cases for the functionality, but one default can result in a silent major bug that can crash an application. The other would never crash without doing something that's clearly unsafe.

But with the default being [weak self] you can silently have a no-op closure, with zero indication of that possibility. You're just swapping one set of possible bugs for another. And in the vast majority of cases, I actually want it to be a strong capture.

The only exception is for handlers that may be called zero or multiple times, such as an event or notification handler. For callbacks, completion handlers, animation blocks, background work, delegation back to the main queue, and most other use cases, the strong reference is the correct solution. As soon as the closure is called, the closure is released, and the reference cycle is broken.

6 Likes

No, the current default will not crash your application. unowned may, but strong won't. It may, however, leak memory and keep invisible view controllers and such around. However, your suggested solution can also result in a silent major bug, and in fact, it often will.

Also, it would be a source breaking change, causing millions of existing Swift programs to alter behaviour, also silently.

Except it wouldn't be silent

thing.doSomething {
    self?.doTheThing()
    // Oh hey, self be nil? maybe I should check for that and throw an error if it shouldn't be!
}

Memory leaks can crash your application

If that's really, true, you're an extreme outlier.

2 Likes

Theoretically, it should be possible to infer wether self should be captured weak or strong without breaking compatibility:
Stick with strong unless there's a self? / if let / guard let... used in the block.

I think I wouldn't like it, though.

It’s true for anyone using closures for offsetting work to a background thread, or passing work back to the main queue. It’s true for animation blocks, network completion handlers, alert dialog button handlers, and everything else where the closure is guaranteed to be called exactly once and then released from memory afterwards.

8 Likes

By default, an unowned reference is a “safe unowned reference”, which guarantees a crash on invalid access – indeed semantically equivalent to force-unwrapping weak. The behaviour you’re describing is unowned(unsafe).

Note that this use of “safe” is consistent with the general Swift usage: safety features are features that avoid undefined behaviour. Crashing when an invariant is violated is well-defined and hence safe; bumbling on when something was unexpectedly nil without knowing what the consequences are is undefined (in terms of application semantics) and hence unsafe.

7 Likes

There seem to be three different things being discussed in this topic.

  1. Boilerplate for guard in a closure.
  2. Boilerplate for capturing self when referencing a function.
  3. Self is captured weak or strong by default.

1. Boilerplate for guard in a closure.

doIt { [guard weak self, guard foo] in
    self.doSomething(foo)
}

Because:

  • guard must be followed by an optional, just self is not optional.
  • guard is required each captured variable that you want to. Ie [guard weak self, foo] is not the same as [guard weak self, guard foo], in the former if foo is nil then the closure body is still executed and the type of foo is optional.
  • It just makes sense to put in the capture variable list. Ask a developer what this means and they can reasonable know what it means.

2. Boilerplate for capturing self when referencing a function.

someAsyncOp(completion: (weak self)?.opFinished)

Because:

  • Doesn't introduce new keywords or concepts, so it's obvious what they do.
  • weak self is just a capture list item. But we don't want to use square brackets here because it's not a list. (and [weak self][0]?.opFinished would be dumb :slight_smile:)

3. Self is captured weak or strong by default.

Capture weakly by default makes more sense because it forces the developer to choose. Either unwrap the optionals or put the strong keyword in the capture list. Currently it's possible for the developer to be oblivious that they have to think about retain cycles and lifetime.

If this was to be changed then the migration tool could just add strong to every capture list item that doesn't have weak to maintain existing behaviour. Alternatively make it an error and make the developer consider what they want to do.

(Making it weak by default would also mean that for 1. the syntax can be [guard self] without the weak)

1 Like
  1. This is also possible if the default is weak self.
  2. And it is almost always incorrect to capture self weakly.
  3. And it is source breaking.

If you think (2) is incorrect, it just goes to prove (1).

1 Like
  1. It definitely takes more thought to use an optional than a non optional as you have to consider the nil case. So while it's still possible to do { self?.foo() } and not realise, it's more obvious than { self.foo() }
  2. Ok please explain further, as I understand it if self has a strong reference to the closure then you want a weak reference to self in the closure to prevent a retain cycle. Or if you don't want to extend the lifetime of self because the closure is not critical or relevant to execute after self would otherwise be deallocated, you use weak.
  3. It's source breaking, but is something the migration tool can handle, just like many other source breaking changes.

My original third point isn't that important I think, it would be much better if swift had some version of point 1 and 2 in my earlier post.

I...what? Is that supposed to be sarcastic? There are so many situations where this is not true that it seems like a joke...sorry if too blunt but I'm genuinely confused.

Maybe if you're working in a codebase with a very specific design...but there are so many design patterns where [weak self] is clearly correct. E.g. any time you make a network call with a callback from a UI Class.

1 Like

It’s true for anyone using closures for offsetting work to a background thread, or passing work back to the main queue. It’s true for animation blocks, network completion handlers, alert dialog button handlers, promises, and everything else where the closure is guaranteed to be called exactly once and then released from memory afterwards.

In the UI case it’s optional to use a weak capture, but it’s clearly not a problem to use the strong default, and it avoids a whole class of bugs where people are just mindlessly guarding on self and returning early.

The worst case scenario in that case, is extra work being performed on a UI component that’s not in the view hierarchy, and that will be released shortly after.

However, for socket event handlers and signal handlers it’s almost certainly best to use weak capture. That’s for closures that are kept around in memory even after they’re invoked, for the purpose of multiple invocation.

3 Likes

That's true from the side of a reference cycle, that's not true from the side of say, a ViewController. I've had bugs/crashes due to this in animation blocks and network closures because the assumption is the ViewController is still on the screen. And while your ViewController is still in memory, you don't have any guarantees the structure/components around it are. So if you do something like pass a function rather than have a [weak self] closure, you can have a lot of weird things happen.

There are also plenty of patterns where this is a major issue. I've had hundreds of MB memory leaks due to this when messing around with singletons and simple messaging systems. Were those poorly designed? Yes :innocent:. But there are plenty of people who will happen upon those patterns out of convenience/ignorance and they may actually be viable with a better developed system around reference capturing.

Sounds like buggy code. Using weak captures doesn’t really fix the underlying problem.

Poorly designed code doesn’t get better with default weak over default strong. You’re just swapping one set of problems for another.

And changing it is source breaking.

1 Like

I'm not defending bad design (especially mine), but the reality is a weak capture does fix a lot of problems like that. It's also a safer route to go. Just because a closure is currently expected to be called once doesn't mean it might not change. IMO [weak self] has fewer long term risks, but that's beside the point.

Whether or not the default should be changed is one thing, but arguing it should almost never be used...? That's certainly a valid (even good) opinion on architecture, but given the amount of people that want better syntax/features around this it's pretty clear it's used enough to be a major pain point.

Just to reiterate, this obviously works when self is used inside bar:
self.foo = { [weak self] in self?.bar($0) }

This is so unswifty, especially when building libraries for consumption. It forces me to make consumers use delegates instead which I don't want to do. I just want consumers to do this:
self.foo = self.bar

The implicit strong reference is no good for these cases, but I imagine would wreak havoc for existing code out there if Swift changed this behind the scenes to implicit weak.

Something like this would be nice:
self.foo = @unowned(self).bar

Or maybe:
@weak func bar() {...}

Apologies for bumping this old thread, but there are tons of dead end discussions about this and is a really rough area in Swift.

5 Likes