0

In the documentation, it says:

The block is copied by the notification center and (the copy) held until the observer registration is removed.

And it provides a one-time observer example code like so:

let center = NSNotificationCenter.defaultCenter()
let mainQueue = NSOperationQueue.mainQueue()
var token: NSObjectProtocol?
token = center.addObserverForName("OneTimeNotification", object: nil, queue: mainQueue) { (note) in
    print("Received the notification!")
    center.removeObserver(token!)
}

Now I expect the observer to be removed as removeObserver(_:) is called, so my code goes like this:

let nc = NotificationCenter.default
var successToken: NSObjectProtocol?
var failureToken: NSObjectProtocol?

successToken = nc.addObserver(
    forName: .ContentLoadSuccess,
    object: nil,
    queue: .main)
{ (_) in
    nc.removeObserver(successToken!)
    nc.removeObserver(failureToken!)

    self.onSuccess(self, .contentData)
}

failureToken = nc.addObserver(
    forName: .ContentLoadFailure,
    object: nil,
    queue: .main)
{ (_) in
    nc.removeObserver(successToken!)
    nc.removeObserver(failureToken!)

    guard case .failed(let error) = ContentRepository.state else {
        GeneralError.invalidState.record()
        return
    }

    self.onFailure(self, .contentData, error)
}

Surprisingly, the self is retained and not removed.

What is going on?

funct7
  • 3,407
  • 2
  • 27
  • 33
  • You would need to break the strong reference to `self` using a capture list like `[weak self]`. Refer to [Strong Reference Cycles for Closures](https://docs.swift.org/swift-book/LanguageGuide/AutomaticReferenceCounting.html#ID56) – keno Jun 27 '19 at 06:55
  • @keno Where is the reference cycle? `self` does not have any reference to the block. – funct7 Jun 27 '19 at 07:06
  • @funct7 `successToken` and `failureToken` are actually properties of `self` right? and you capture `self` inside the blocks, so retain cycle happens – HSG Jun 27 '19 at 07:12
  • @HSG Nope. Note the `var` keyword before each token. – funct7 Jun 27 '19 at 07:12
  • My suspicion would be the tokens. They are declared as optionals belonging to `self` -- my assumption is that all this code is part of a class declaration. When the observer is added, `self` is referenced in the closure which creates a strong reference. Even though you remove the observers in the closure, they are still "alive" until deallocated with a nil assignment. – keno Jun 27 '19 at 07:15
  • @funct7 I see. May you post more code, and I am curious what is `onSuccess` and `onFailure`, are they delegate or sth? – HSG Jun 27 '19 at 07:15
  • @HSG They are closure properties on the `self` in the code. They are accessed via `self` so they shouldn't really come in to play, right? :/ `.contentData` is just an `enum`. – funct7 Jun 27 '19 at 07:18
  • @HSG Please check my answer. – funct7 Jun 27 '19 at 11:37

3 Answers3

0

Confirmed some weird behavior going on.

First, I put a breakpoint on the success observer closure, before observers are removed, and printed the memory address of tokens, and NotificationCenter.default. Printing NotificationCenter.default shows the registered observers.

I won't post the log here since the list is very long. By the way, self was captured weakly in the closures.

Printing description of successToken:
▿ Optional<NSObject>
  - some : <__NSObserver: 0x60000384e940>
Printing description of failureToken:
▿ Optional<NSObject>
  - some : <__NSObserver: 0x60000384ea30>

Also confirmed that observers were (supposedly) removed by printing NotificationCenter.default again after the removeObserver(_:)s were invoked.

Next, I left the view controller and confirmed that the self in the quote code was deallocated.

Finally, I turned on the debug memory graph and searched for the memory addresses and found this:

enter image description here

In the end, there was no retain cycle. It was just that the observers were not removed, and because the closures were alive, the captured self was alive beyond its life cycle.

Please comment if you guys think this is a bug. According to the documentation on NotificationCenter, it most likely is...

funct7
  • 3,407
  • 2
  • 27
  • 33
0

Recently I've run into similar problem myself.

This does not seem a bug, but rather undocumented feature of the token which (as you've already noticed) is of __NSObserver type. Looking closer at that type you can see that it holds the reference to a block. Since your blocks hold strong reference to the token itself (through optional var), you have a cycle.

Try to set the optional token reference to nil once it is used:

let nc = NotificationCenter.default
var successToken: NSObjectProtocol?
var failureToken: NSObjectProtocol?

successToken = nc.addObserver(
    forName: .ContentLoadSuccess,
    object: nil,
    queue: .main)
{ (_) in
    nc.removeObserver(successToken!)
    nc.removeObserver(failureToken!)

    successToken = nil // Break reference cycle
    failureToken = nil

    self.onSuccess(self, .contentData)
}
deekay
  • 899
  • 6
  • 8
-1

You need to use weak reference for selflike this :

let nc = NotificationCenter.default
var successToken: NSObjectProtocol?
var failureToken: NSObjectProtocol?

successToken = nc.addObserver(
    forName: .ContentLoadSuccess,
    object: nil,
    queue: .main)
{[weak self] (_) in

    guard let strongSelf = self else { return }

    nc.removeObserver(successToken!)
    nc.removeObserver(failureToken!)

    strongSelf.onSuccess(strongSelf, .contentData)
}

failureToken = nc.addObserver(
    forName: .ContentLoadFailure,
    object: nil,
    queue: .main)
{[weak self] (_) in

    guard let strongSelf = self else { return }

    nc.removeObserver(successToken!)
    nc.removeObserver(failureToken!)

    guard case .failed(let error) = ContentRepository.state else {
        GeneralError.invalidState.record()
        return
    }

    strongSelf.onFailure(strongSelf, .contentData, error)
}
Coder ACJHP
  • 1,940
  • 1
  • 20
  • 34
  • No but where is the reference cycle created, if there is one? – funct7 Jun 27 '19 at 07:07
  • Why down vote!! `self.onSuccess(self, .contentData) ` this is a reference for self – Coder ACJHP Jun 27 '19 at 07:08
  • The block captures `self`, the block is captured by `NotificationCenter`. There is no cycle. A reference per se does not create a retention cycle. – funct7 Jun 27 '19 at 07:10
  • The solution is correct, and I know the solution. The question is why is there an unexpected retention of `self`? – funct7 Jun 27 '19 at 07:11
  • And you may remove the retained reference with: `deinit { NotificationCenter.default.removeObserver(self) }` – Coder ACJHP Jun 27 '19 at 07:14
  • Remove it form who is the observer – Coder ACJHP Jun 27 '19 at 07:16
  • I'm sorry, but how does that work? `self` was never added as an observer. The observer in the code is the closure/block. – funct7 Jun 27 '19 at 07:49
  • Functions and closures are first-class citizens in Swift because you can treat then like a normal value. For example, you can assign a function/closure to a local variable or pass a function/closure as an argument or return a function/closure so in your code your class (class that you declared this closures) will be the owner. Double check your code you passing parameter with result inside closure from observer class – Coder ACJHP Jun 27 '19 at 09:14
  • Yeah the result is a class-type local variable, not a propery on the class. The fact that closures are first-class citizens has nothing to do with the closure’s context capturing. – funct7 Jun 27 '19 at 09:49
  • The observer—the closure—captures the token and `self`. `NotificationCenter` holds the closure until `removeObserver(_:)` is called. Where is the cycle? For a retain cycle to happen, `self` should have a strong reference on the closure. – funct7 Jun 27 '19 at 09:52