Is it the right way using `[weak self]` in swift closure?

Question

I always using [weak self] in swift closure to prevent reference cycle. Here is the code below, is it the correct way?

someTask(completion: {[weak self] (result) in
        if self == nil {  
            return
        }

        //is it safe when reach here? 

        self!.xxx = yyy
        self!.doLongTermWork()
        self!.finish()  //will crash when self is nil?
    })

Weak self does not keep a strong hold on the instance. So when self.doLongTermWork(), will self be set to nil again somewhere else?

Answer 1

Your pattern has race condition. If self was deallocated at the exact same time as your completion handler closure was executing, it could crash. As a general rule, avoid using the ! forced unwrapping operator if you can.

1. I’d lean towards the guard “early exit” pattern (reducing nested braces, making code easier to read). The standard Swift 4.2 solution is:

   someTask { [weak self] result in
       guard let self = self else { return }
   
       self.xxx = yyy
       self.doLongTermWork()
       self.finish()
   }
   

2. Before Swift 4.2, which implemented SE-0079, we would have to do something like:

   someTask { [weak self] result in
       guard let strongSelf = self else { return }
   
       strongSelf.xxx = yyy
       strongSelf.doLongTermWork()
       strongSelf.finish()
   }
   

   You can see why we prefer the Swift 4.2 improvement, as this strongSelf syntax is inelegant.

3. The other obvious alternative is just:

   someTask { [weak self] result in
       self?.xxx = yyy
       self?.doLongTermWork()
       self?.finish()
   }
   

   Sometimes you need the “weak self - strong self dance” (the first two alternatives), but it would not appear to be the case here. This is likely sufficient.

There are other scenarios/edge cases that one might contemplate, but these are the basic approaches.

Answer 2

You said:

someTask(completion: {[weak self] (result) in
    if self == nil {  
        return
    }
    //is it safe when reach here? 
    self!.xxx = yyy
})

No! You have not retained self, so in theory it might become nil at any time during the execution of the closure. It probably won't, but "probably" is not good enough. And exclamation marks are always an invitation to crash.

Do the weak-strong dance, and do it correctly:

someTask(completion: {[weak self] (result) in
    if let self = self {  // or let `self` before Swift 4
        // here, self is safe, because you made the reference strong again
        self.xxx = yyy
    }
})