Temporary solution to weak strong dance

[chrisbia]

I'm faced with a lot of instances where I want to capture a parameter weakly in a closure and am forced to check if it exists before executing, and if not it becomes a non-op:

self.storedProperty.add(closure: { [weak self] in
    guard let self=self else { return }
    self.doStuff()
})

Often times these types of closures do not contain much code, which means for the sake of readability I have to turn a oneliner into at a minimum a threeliner — if you want to add a semicolon after the guard.

An if statement could work as well:

self.storedProperty.add(closure: { [weak self] in
    if let self=self else { self.doStuff() }
})

But I just feel like there is a superior way that warrants consideration:

self.storedProperty.add(closure: { [weakStrong self] in self.doStuff() })

I think this latter syntax is very clear in its intent. We all know the 'weak/strong' dance. So simply using it literally as the reference modifier seems intuitive if that's what you're trying to do. In terms of simplicity, I think it's clear that all this does is only execute the closure if the weakStrong reference exists. I don't know how difficult this would be to implement, but at some level, all it really would be doing is adding a guard to check for the existence of the reference type at the beginning of the closure.

This appears to be a purely additive addition to the language. That said, I have seen a few posts that criticize capturing self in the first place, and some that expressed entirely new ways of doing this sort of thing that syntactically, didn't even involve closures. I'm not aware of the path the Swift developers are taking in regards to this type of usage, but until it is traversed and we have that alternative — this is why I say "temporary" — I feel that this syntax offers a lot to be gained while requiring relatively little to gain it.

[sveinhal (Svein Halvor Halvorsen)]

Isn’t this just as short, and more clearly communicates the fact that the closure is a potential no-op:

self.storedProperty.add { [weak self] in
    self?.doStuff()
}

I think we should be very weary of adding new conveniences to the language that are potential no-ops, and thus may hide subtle bugs.

More often than not, a strong capture is really what you want (completion handlers, animation blocks, dispatches to other queues, alert button handlers, and other single-invocation closures).

And when you have multi-invocation closures such as signal handlers, event handlers, their lifetime is often tied to self, and you can use unowned capture.

In other cases, it’s better to be explicit about how to handle nil self, methinks.

[lassejansen]

I've solved this issue for myself by using a special container type that can store a weakly held object alongside the closure. The trick that is used by the container type is to check if the object still exists before calling the closure. If the object still exist, it is passes it as a non-optional parameter to the closure. If it doesn't exist anymore, the closure isn't called at all.

As an example, consider a ViewController with a button that has an onTap handler:

class ViewController: UIViewController {

    let button = MyButton()

    override func viewDidLoad() {
        (...)
        button.onTap(with: self) { vc in // only called if self still exists
            vc.doSomething()
        }
    }
}

To make this work MyButton uses the CallbackStore container type:

class MyButton {
    let onTap = CallbackStore<()>() // In this example we don't pass an extra parameter to the callback closure, hence the () type parameter

    func touchUpInside() {
        onTap.executeAll() // Executes all callback closures
    }
}

Incidentally, the implementation of CallbackStore contains the first (and so far only) use I found for callAsFunction.

CallbackStore.swift

public class CallbackStore<P> {

    fileprivate var callbacks: [CallbackStoreItem]

    public init() {
        callbacks = []
    }

    public func callAsFunction<O: AnyObject>(with object: O, block: @escaping (O) -> ()) {
        callbacks.append( WeakItem(owner: object, block: block))
    }

    public func callAsFunction<O: AnyObject>(with object: O, block: @escaping (O, P) -> ()) {
        callbacks.append( WeakParameterItem(owner: object, block: block))
    }

    public func callAsFunction(block: @escaping () -> ()) {
        callbacks.append( StrongItem(block: block))
    }

    public func callAsFunction(block: @escaping (P) -> ()) {
        callbacks.append( StrongParameterItem(block: block))
    }

    public func executeAll(value: P) {
        callbacks.forEach { $0.execute(value: value) }
    }
}

extension CallbackStore where P == () {
    public func executeAll() {
        executeAll(value: ())
    }
}

public class CallbackStore2<P1, P2>: CallbackStore<(P1, P2)> {
    public func callAsFunction<O: AnyObject>(with object: O, block: @escaping (O, P1, P2) -> ()) {
        callbacks.append( WeakParameterItem(owner: object) { (object: O, tuple: (P1, P2)) in
            block(object, tuple.0, tuple.1)
        })
    }

    public func callAsFunction(block: @escaping (P1, P2) -> ()) {
        callbacks.append( StrongParameterItem(block: { (tuple: (P1, P2)) in
            block(tuple.0, tuple.1)
        }))
    }
}

public class CallbackStore3<P1, P2, P3>: CallbackStore<(P1, P2, P3)> {
    public func callAsFunction<O: AnyObject>(with object: O, block: @escaping (O, P1, P2, P3) -> ()) {
        callbacks.append( WeakParameterItem(owner: object) { (object: O, tuple: (P1, P2, P3)) in
            block(object, tuple.0, tuple.1, tuple.2)
        })
    }

    public func callAsFunction(block: @escaping (P1, P2, P3) -> ()) {
        callbacks.append( StrongParameterItem(block: { (tuple: (P1, P2, P3)) in
            block(tuple.0, tuple.1, tuple.2)
        }))
    }
}

// Private types

fileprivate protocol CallbackStoreItem {
    func execute(value: Any)
}

fileprivate struct WeakItem<O: AnyObject>: CallbackStoreItem {
    weak var owner: O?
    let block: (O) -> ()

    func execute(value: Any) {
        if let owner = owner {
            block(owner)
        }
    }
}

fileprivate struct WeakParameterItem<O: AnyObject, T>: CallbackStoreItem {
    weak var owner: O?
    let block: (O, T) -> ()

    func execute(value: Any) {
        if let owner = owner, let value = value as? T {
            block(owner, value)
        }
    }
}

fileprivate struct StrongItem: CallbackStoreItem {
    let block: () -> ()

    func execute(value: Any) {
        block()
    }
}

fileprivate struct StrongParameterItem<T>: CallbackStoreItem {
    let block: (T) -> ()

    func execute(value: Any) {
        if let value = value as? T {
            block(value)
        }
    }
}

[Dmitriy_Ignatyev]

Thanks for writing this pitch, the idea is great.

I have just searched my current project, and I have found 633 writings of 'weak self' and 79 writings of
'guard let self = self'.
There are 2 points to clarify here:

1. It will be good to find alternative spellings to 'weakStrong'. For example [weak/strong self], [weak\strong self], [weak->strong self], [weak strong self], [weak, strong self] ...

2. What should we do, keeping in mind such code:

let deactivatePromoCode = { [weak self] (orderId: UInt64, completion: @escaping PromoCodeCompletion) in
  guard let self = self else {
    perform(completion: completion, withError: Self.internalError, on: .global(qos: .userInitiated))
    return
  }
  
  self.networkProvider.deactivatePromoCode(forOrderId: orderId) { [weak self] result in
    guard let self = self, case .promoCodeOperation = self.state else {
      perform(completion: completion, withValue: result, on: .global(qos: .userInitiated))
      return
    }
    completion(result)
  }
}

Inexperienced users will be using weakStrong by default because it is convenient. But in the example above completion must be called with error/result, if self was deallocated. Using of weakStrong here will lead to floating bugs, because sometimes completion will not be executed, which is unexpected.
I know this is very rare case. So this feature for example can only be allowed if closure has no closures in its arguments.
Any thoughts about this?

[sveinhal (Svein Halvor Halvorsen)]

But why do the weak/strong dance at all in the latter/inner call?

The network call is guaranteed to run to completion. For exactly-once called closures, there should seldom be any need for weak capture. The strong capture will only delay deinit of self by at most a few seconds.

For your first closure, it is hard to tell if you really need the weak capture, but I suspect you don’t, for the same reasons.

It is my experience that this is often the case. That people ask for more ergonomic weak/strong syntax, when the real solution is just to use the default capture with no special syntax and be done with it.

Granted, Combine, with its multi-invocation signal handlers, has made the problem more common, but it is still quite rare.

[clayellis]

This has already been discussed at length:

• Easy strongified weak `self` in closures - #15 by Jon_Shier
• [Proposal] Allow upgrading weak self to strong self by assignment - #9 by Shawn_Erickson
• Proposal: weakStrong self in completion handler closures
• Guarded closures - #10 by acecilia
• [Draft] Guarded closures and `@guarded` arguments
• $self

[lassejansen]

@sveinhal

It is my experience that this is often the case. That people ask for more ergonomic weak/strong syntax, when the real solution is just to use the default capture with no special syntax and be done with it.

While it's true that this often works, you still have to spend mental energy to decide if this would create a memory leak and if you need an unowned or weak annotation. In general it is of course a good idea to think about retain cycles in callbacks, but especially in view-related code it's getting boring after a few hundred callbacks, and it's also error-prone.

Consider this example:

override func viewDidLoad() {
    super.viewDidLoad()

    button.onTap { [unowned self] in
        self.dataSource.refresh()
    }

    dataSource.onRefresh { result in
        switch result {
        case .failure(let error):
            self.showError(error)
        case .success(let data):
            self.updateView(data: data)
        }
    }
}

func updateView(data: AppData) {
    UIView.animate(withDuration: 0.25) {
        // do some animations
    } completion: { _ in
        self.tidyUp()
    }
}

I see the following problems with this approach:

1. If you forget the [unowned self] annotation in the button.onTap handler you get a memory leak.
2. If you mistakenly add an [unowned self] to the dataSource.onRefresh-Handler, you get a crash if the view controller gets dismissed while the refresh is running.
3. If you mistakenly add an [unowned self] to the animation-completion handler, you get a crash if the view controller gets dismissed while the animation is running.
4. If you close the view controller while the refresh is running, showError or updateView (including the animation) is called unnecessarily.
5. If you close the view controller while the animation is running, tidyUp is called unnecessarily.

This is why many developers (including me) come to the conclusion that it's a more robust approach to always use the [weak self] annotation in view-related code and to always write callback code with the assumption that it will not be executed if the view was closed.

The problem with this is (as OP and many others have already written) that the [weak self] annotation together with the optionality of self is quite annoying in this case.

The point I tried to make earlier in this thread is though that this issue can be solved with a library, without additional language support. The only disadvantage is that you need to wrap APIs that you didn't write yourself.

The converted example would look like this then:

override func viewDidLoad() {
    super.viewDidLoad()

    button.onTap(with: self) { $0.dataSource.refresh() }

    dataSource.onRefresh(with: self) { vc, result in
        switch result {
        case .failure(let error):
            vc.showError(error)
        case .success(let data):
            vc.updateView(data: data)
        }
    }
}

func updateView(data: AppData) {
    UIView.animate(with: self, duration: 0.25) { vc in
        // do some animations
    } completion: { vc, _ in
        vc.tidyUp()
    }
}

[sveinhal (Svein Halvor Halvorsen)]

Sure. But imho people vastly overuse the weak capture, to the point where it often leads to buggy code. Most times people ask for more ergonomic syntax for weak/strong, the better solution is usually to avoid weak capture at all.

One undulating only need to consider a single thing, when capturing self: “Will this closure be executed multiple times?”

I.e will it be stored indefinitely.

Your examples are both event handlers. We may see more of those with the proliferation of Combine, but I still thing people overuse weak.

In your examples, it’s not clear from the call site that the argument is captured weakly, and also not clear what happens if the argument is nulled by the time the closure is called.

[lassejansen]

@sveinhal

One undulating only need to consider a single thing, when capturing self: “Will this closure be executed multiple times?”

Sure, but it's still an easy mistake to make, and one that can be avoided by always defaulting to weak in view related code.

@sveinhal

Your examples are both event handlers. We may see more of those with the proliferation of Combine, but I still thing people overuse weak.

Even in my UIKit projects ~80% percent of the callbacks are event handlers. But it's probably different for Storyboard based code.

@sveinhal

In your examples, it’s not clear from the call site that the argument is captured weakly, and also not clear what happens if the argument is nulled by the time the closure is called.

The closure is only executed if the argument hasn't been nulled, otherwise it isn't called at all. If it still exist the argument is passed as a non-optional first argument to the closure (see my first comment in this thread for the implementation).

The with: label is supposed to communicate that the closure is only called if the argument still exist, but there are probably better names.

@sveinhal

But imho people vastly overuse the weak capture, to the point where it often leads to buggy code. Most times people ask for more ergonomic syntax for weak/strong, the better solution is usually to avoid weak capture at all.

For view-related code I use weak captures exclusively and I haven't noticed any bugs due to this. What kinds of bugs are you thinking of?

The worst bugs I've seen were mostly due to retain cycles: Long-dismissed zombie view controllers still listening to observers and messing with application state.

[lassejansen]

@sveinhal

Your examples are both event handlers.

On the first read of your comment I thought that was wrong, but didn't comment. But you are right of course. I initially thought of the dataSource.onRefresh closure as a one-time closure that's only executed once after the user hit the refresh button and the refresh finished. In an earlier version of the example I had it as a completion handler parameter on the refresh function, and then "refactored" it into it's own handler on the dataSource object—and by doing that I unintentionally created a memory leak.

I don't know if that counts, but it kind of proves my point about strongly capturing self being very error prone Always defaulting to weak capture would have prevented this bug.

[Tino]

The weak-strong dance is probably one of the most annoying aspects of Swift — but it has always been that way, and I doubt that after all those years someone will come up with a "solution".

Possible options

Breaking compatibility isn't a real option anymore, so there is a quite narrow corridor when you don't want to accept other compromises.
Inferring weak automatically when the body of the closure indicates that (self?.doSomething(), guard let self = self…) could be an option, but I wouldn't push toward that direction (too much magic); a second way to specify closures (without deprecating the current one) wouldn't be perfect either…

At least partially, it's not even a problem of how closures are used, but rather where:
People tend to hype and overuse "new" features, and I see blocks in that phase — which is usually followed by a backswing, so I wouldn't be surprised if future developers look at closures, async & await or protocols in the same way as some coders look at subclassing or global variables now (and they'll be as wrong as the extremists today).

Take Network as an example, and search for the words "A handler that" in the documentation of basic types (NWListener, NWConnection).
All those handlers create a high risk for retain cycles, and can't I see any real advantage of this design. Some years ago, it might have been implemented as a delegate, and I'll go even one step further and say that subclassing would actually be the best choice: It is simple (only one object), and there is little danger to use it wrong.

There are places to use "modern" features like closures, but I neither believe in silver bullets, nor do I follow dogmas like "never use inheritance".

Bottom line: Choose your tools wisely, and when closures cause problems, there might be alternatives which actually perform better (of course, this advice doesn't help with existing API ).

[Dmitriy_Ignatyev]

"For your first closure, it is hard to tell if you really need the weak capture, but I suspect you don’t, for the same reasons." – I need it to be weak 100%, because user can close screen, screen module will be detached, but Interactor will still be alive, which is memory leak.

[sveinhal (Svein Halvor Halvorsen)]

@lassejansen

What kinds of bugs are you thinking of?

The most common bug is related to weak self and guard-return, is completion handlers that are never called.

Hopefully, the new async/await syntax will turn callback based functions into (asynchronously) returning functions, and naive guard-returns will be caught by the “returning void from non-void” function error.

I guess it could also be solved using some kind of static analysis of completion handler arguments, that made sure completion handlers were called on every path, similar to the current return statement analysis.

But as it stands, today, missing calls to callbacks are a great cause of bugs in many programs.

[sveinhal (Svein Halvor Halvorsen)]

Maybe you’re right. I don’t know your code. But your inner closure does not need it at least. In fact, it makes your code much harder to read and reason about.

[lassejansen]

@sveinhal

The most common bug is related to weak self and guard-return, is completion handlers that are never called.

Ok, I see where you are coming from. I think it would make sense to split the problem into two use cases (as you did before already, I think):

1. View-related callbacks / event handlers: These callbacks are usually executed on the main thread and (hopefully) don't contain any global business logic. The main source of bugs are retain cycles.
2. Business-logic callbacks: Successions of callbacks that can run on background threads and that must ensure that a completion handler is either executed or passed to the next function. The main source of bugs are completion handlers that are never called.

The way I usually handle the latter problem is by wrapping all completion handler APIs into blocking calls and running them on background queues. Which means that almost all completion handlers in my apps (ignoring the wrapper code) are event handlers that ideally capture self weakly. IMO the event handler use case would deserve better language support.

[lassejansen]

@Tino

The weak-strong dance is probably one of the most annoying aspects of Swift — but it has always been that way, and I doubt that after all those years someone will come up with a "solution".

I think the solution that most people want is what @chrisbia proposed:

1. A way to prevent a closure from being executed if self is captured weakly.
2. That self isn't wrapped in an optional within the closure.

The proposed spelling of [weakStrong self] is a little unfortunate, because it doesn't communicate that the closure may not be executed. If something like this would be added to the language I would prefer the spelling [if self]:

dataSource.onRefresh { [if self] result in
    self.handleResult( result)
}

[Tino]

The problem of this concept is that it has been discussed long ago, and several times — without acceptance (@clayellis already collected some links).
One weak spot of the idea itself is that there are not only void-returning closures. This could also be solved, but makes everything slightly more complicated...

[Dmitriy_Ignatyev]

Seems that async / await will be implemented earlier than this feature. And when async / await will appear, weakStrong will not be needed .

[Jon_Hull]

I generally agree that the correct spelling might actually help this to be intuitive to the reader. [if self] is nice in that it is short.

Other options:
• [guard self]
• [guarded self]
• [require self]
• [check self]