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.