So back in the dark ages, we registered to receive notifications like this:

-[NSNotificationCenter  addObserver:selector:name:object:]

In other words, the target-action pattern. When the notification is received, call this selector (action) on this target. And all was well.

Then in iOS 4, blocks (closures) were added to iOS. And it was the hip cool thing to add block versions of everything. Blocks in a box! Blocks in their socks! After all, the NSNotification (observer) pattern is sort of deferred execution, right?

So a blocks-based API was added to NSNotificationCenter. Here it is:

-[NSNotificationCenter addObserverForName:object:queue:usingBlock:]

This turned out to be a terrible idea. This could be, I think, the single biggest API blunder in iOS. (Except maybe iCloud Core Data.) I have debugged issues that were root-caused to this misleading API more than ten times. I have lost more than four weeks to this API. I have no less than six radars open about it.

All of this is in spite of the fact that it is banned from any codebase I work on.

For those who accuse me of drinking the Apple kool-aid, rest assured, there is nothing pro-Apple in this blog post. Read on for 2500 words of pure complaining.

So how bad could it be?

We’re going to be good little TDD citizens for the duration of this blog post, so let me show you the test first:

- (void)testExample
{
    for(int i =0; i < 5; i++) {
        YourAttempt *attempt1 = [[YourAttempt alloc] init];
        [[NSNotificationCenter defaultCenter] postNotificationName:notificationName object:nil];
        XCTAssertEqual(counter, i+1, @"Unexpected value for counter.");
        XCTAssertEqual(1, attempt1.localCounter, @"Unexpected value for localCounter.");
    }
}

So this test is trivial:

• We create an Attempt object
• We post a notification
• We check that the notification increases our global counter variable.
• We check that the notification increases the object’s localCounter variable.

Now at this point you’re probably saying: “This looks sort of simple.” If that is you, just skip the rest of this blog post, pull down the GitHub repository, type your solution into YourAttempt.m, and press Command-U. But not until you’re really, really sure it’s right. Don’t worry, I’ll wait. It’ll be more fun if you fail first.

Still reading? You lazy bum. Making me do all the work.

If at first you don’t succeed

Our first attempt really is trivial:

@interface Attempt1() {

}
@end

@implementation Attempt1
-(id)init {
    if (self = [super init]) {
        [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            self.localCounter++;
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

        }];
    }
    return self;
}
@end

That should do the trick! Except:

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

Can you figure out what’s going on here? We’re looking for 1,2,3,4,5 in our counter —but we’re getting 1,3,6,10,15. So why those numbers?

Well, that sequence is called the triangular numbers. And that pattern is a dead giveaway. See, the first time we post the notification, it runs once. So our counter is 1. But the second time, it runs twice. So our counter is 3. And the third time, it runs thrice. Now our counter is six.

Now you might say: “So what if my notification runs a few times too many? I’m not stupid enough to use global variables.” Well, maybe. If you’re using things like the camera, the microphone, anything in your app delegate, etc., then you are, actually, using global variables. But forget about that for a minute–imagine what would happen if we picked a function at random from your codebase and ran it twice instead of once. What would happen? We could try to insert two objects into your database, or delete an object twice. We could pop an extra view controller. We could repeat your online purchase. Who knows?

In fact, the really dangerous thing about this bug is that practically anything can happen. This bug can present as any bug. Any bug report you get can be this bug. That’s bad. That’s really bad. And that’s why I have spent so much time on bugs that turned out to be this one. Just to give some real-life bug reports:

• “Whenever I try to take a picture, the lens doesn’t open.”
• “If I go to Screen A, leave it, and come back, the button on Screen B does something really strange.”
• “After I pick a photo from my photo library, the app works fine. For about 20 seconds. Then it crashes. But I can only reproduce this once per testing session. I have to wait until tomorrow to catch it again.”

Do these sound like repeated notification bugs? No. That’s why it’s so scary.

So: let’s not get notifications twice. Well, obviously! We forgot to unregister for the notification. Let’s do that.

A very selfish attempt

@interface Attempt2() {
    id cleanupObj;
}
@end

@implementation Attempt2
-(id)init {
    if (self = [super init]) {
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            self.localCounter++;
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

        }];
    }
    return self;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}

@end

Run that and… bam! We’re done!

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

…what? The exact same result? What’s going on here?

Well, the same thing as last time. Even though we have written code in our dealloc to remove our Attempt from the notification, the notification keeps getting called. So why? Did we get the syntax wrong or something?

No, the syntax is fine. What’s wrong is that dealloc never gets called. Why not?

Well, when you declare a block, the compiler goes in and scans what the block does. This is because if you write some code like id x = @(42); and then declare a block that uses x, that block might long outlive the variable x. So x needs to stick around at least as long as the block does.

So the culprit here is that our block contains the expression

self.localCounter++;

which is syntactically equivalent to

[self setLocalCounter:[self localCounter]+1];

Which contains not one, but two references to self. So the block, as soon as it is declared, acquires an owning reference to self, because the block needs self to run. And since NSNotificationCenter owns the block, and the block owns self, self will never be deallocated. Ever.

Hey, you want to know what else is scary? This code builds cleanly. Not a peep from the compiler; not a peep from Clang Static Analyzer. In fact, every buggy code listing you see in this post gets a clean bill of health from both. This is in spite of the fact that LLVM has a warning for this very bug. You might have seen it:

Capturing self strongly in this block is likely to lead to a retain cycle

Clang is just not powerful enough, in its present form, to find this type of bug. Consider yourself alarmed.

Well, the solution is trivial: we just remove self from our block.

Practicing selflessness

@interface Attempt3() {
    id cleanupObj;
}
@end
@implementation Attempt3
-(id)init {
    if (self = [super init]) {
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            _localCounter++;
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

        }];
    }
    return self;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}
@end

And… tada!

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

Wait, still?

Yeah, still. So this is actually the same problem as before, it’s just implicit. You see, if we’re going to keep the _localCounter ivar around, we also have to keep self around to put it in.

Thus sayeth the documentation:

When a block is copied, it creates strong references to object variables used within the block. If you use a block within the implementation of a method [and] you access an instance variable by reference, a strong reference is made to self

The documentation goes on to suggest:

To override this behavior for a particular object variable, you can mark it with the __block storage type modifier.

Ah, so that should be straightforward! We’ll just mark the _localCounter with __block.

It’s a __block party

@interface Attempt4() {
    id cleanupObj;
    __block int _localCounter;
}
@end
@implementation Attempt4
-(id)init {
    if (self = [super init]) {
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            _localCounter++;
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

        }];
    }
    return self;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}
@end

How bad could it be?

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

Hmm… ok, now you’re just messing with me, Apple. You’ve told me doing this would solve my problem. What gives?

What gives is that this documentation flits back and forth between whether or not it’s talking about an object variable, as opposed to, I guess, the other kind. See:

it creates strong references to object variables used within the block… If you access an instance variable by reference, a strong reference is made to self;… To override this behavior for a particular object variable, you can mark it with the __block storage type modifier.

In other words, at the beginning and (crucially) the workaround, they’re talking about object variables. Meanwhile what we’re using is just an integer.

OK so maybe we can convert our code to use an object variable, and then the workaround will work?

When the documentation fails

@interface Attempt5() {
    id cleanupObj;
    __block NSNumber *counterObj;
}
@end
@implementation Attempt5
-(id)init {
    if (self = [super init]) {
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            counterObj = @(counterObj.intValue + 1);
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

        }];
    }
    return self;
}

- (void)setLocalCounter:(int)localCounter {
    counterObj = @(localCounter);
}

- (int)localCounter {
    return counterObj.intValue;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}
@end

And with a Cmd-U, we’ll get…

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

Seriously? How did this get through QA? Did anyone even test this? What sort of person even wrote this documentation?

Well, the sort of person who doesn’t read compiler specifications. Noobs. Because, in-between WWDC videos, documentation, sample code, and you know, actually writing code, I’ll bet you have nothing better to do than browse random technical specs on clang.org.

Because a 27-page document that doesn’t even rate a mention in the Clang documentation table of contents very clearly states buried in the middle of Section 7.5:

The inference rules apply equally to __block variables, which is a shift in semantics from non-ARC, where __block variables did not implicitly retain during capture.

Go ahead and try to make sense of that sentence, I dare you.

No? So essentially this is compilerese for “We changed it.”

Back in the Land Before ARC, the use of __block would prevent a block from retaining a variable. However in the ARC world, we have a whole constellation of memory keywords: __strong, __weak, __autoreleasing, __unsafe_unretained… And so when they introduced these, they decided to divorce __block from the memory keywords, so you could have, say, __unsafe_unretained __block id foo; if you like. And just like every other kind of variable, the default, implicit memory keyword for __block is __strong.

So that’s why it doesn’t work. Now, you might say, let’s just declare __counterObj as memory type __weak. But of course, then it would have no strong reference, and vanish into the æther. Nor can we create a weak reference to our counter object immediately prior to the block and use that, because we must eventually set our counter to a new value.

So instead we will improvise:

Your invariants may vary

@interface Attempt6() {
    id cleanupObj;
}
@end
@implementation Attempt6
-(id)init {
    if (self = [super init]) {
        __weak Attempt6 *mySelf = self;
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");
            mySelf.localCounter++;
        }];
    }
    return self;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}
@end

There are a lot of things not to like about this solution: it relies on using a public interface to access our own members, for example. So any notification you process will need to use an API with public scope visibility, which means you will have to expose one. And hope that nobody uses it.

But anyway, it should work, right? (Or have you figured out this gimmick by now?)

"3" is not equal to "2" - Unexpected value for counter.
"6" is not equal to "3" - Unexpected value for counter.
"10" is not equal to "4" - Unexpected value for counter.
"15" is not equal to "5" - Unexpected value for counter.

SERIOUSLY. MUST. KILL. COMPILER.

OK, what is wrong here? I’ll give you a hint: if you test in Release mode, it works fine. It only fails in Debug mode.

Give up?

Here’s the answer:

NSAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");

See, NSAssert is a macro. A macro that expands to this:

do {                
    __PRAGMA_PUSH_NO_EXTRA_ARG_WARNINGS 
    if (!(condition)) {     
        [[NSAssertionHandler currentHandler] handleFailureInMethod:_cmd 
        object:self file:[NSString stringWithUTF8String:__FILE__] 
            lineNumber:__LINE__ description:(desc), ##__VA_ARGS__]; 
    }               \
        __PRAGMA_POP_NO_EXTRA_ARG_WARNINGS 
} while(0)

See there? Big fat self. Ergo, retain cycle, ergo test failure.

The Final Solution

Here’s the final answer, using the lesser-known NSCAssert function. Which, by the way, is not supposed to be used in Objective-C:

This macro should be used only within C functions.

But hey, we’ve already demonstrated that this documentation is buggier than MobileMe on launch day.

@interface Attempt7() {
    id cleanupObj;
}
@end
@implementation Attempt7
-(id)init {
    if (self = [super init]) {
        __weak Attempt7 *mySelf = self;
        cleanupObj = [[NSNotificationCenter defaultCenter] addObserverForName:notificationName object:nil queue:nil usingBlock:^(NSNotification *note) {
            int oldCounterValue = counter;
            counter++;
            NSCAssert(counter==oldCounterValue+1, @"Atomicity guarantee violated.");
            mySelf.localCounter++;
        }];
    }
    return self;
}

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:cleanupObj];
}
@end

So now looking at a working implementation, is this the sort of thing you would come up with left to your own devices? I’m guessing no.

And you’re in good company

The documentation lists no fewer than six sample code projects underneath the Notifications-with-blocks API. And five out of six Apple model projects are wrong. Let that sink in.

• AVCam – On line 89 of AVCamCaptureManager.m, Apple uses the syntax __block id weakSelf = self;, apparently under the impression that this breaks the reference cycle. No instance of this class can ever be deallocated. A GitHub search reveals some 77 repositories contain some version of this class.
• AVLoupe – In this project, Apple straight-up never removes the observer from the NSNotificationCenter. How’s that for sample code?
• AVMovieExporter’s VideoLibrary crashes on dealloc for unrelated reasons. Bonus points are awarded for a novel use of __unsafe_unretained __block to work around the retain cycle that will cause an EXC_BAD_ACCESS crash if the retain cycle is broken and a notification is subsequently posted.
• AudioTapProcessor – boldly using self inside the block. Bonus points awarded for attempting to remove the observer in viewDidUnload in spite of the fact that viewDidUnload is never called in iOS 7, which was in beta 5 when this code was published. In fairness, it might work on iOS 6. I didn’t test it.
• TaggedLocations (a.k.a. SimpleCoreDataRelationships) is the only project I can’t find anything wrong with. So good job, anonymous developer of this sample project, you’re in the top 13% of Cocoa engineers at Apple.

Killing the beast

For the rest of us, just stay far away from this API. cd into your code’s directory and type this to find out how bad your codebase is:

grep -R "addObserverForName:.*object:.*queue:.*usingBlock.*" --include="*.m" .

This API is banned from my codebases. There is certain third-party code where I will tolerate grandfathering it in, but not without a thorough audit. And the audit, unfortunately, usually finds something.

Alternatives

Nick Lockwood has been working on an alternative API over here.

Jonathan Willing has an alternative API over here.

---

Want me to build your app / consult for your company / speak at your event? Good news! I'm an iOS developer for hire.

Like this post? Contribute to the coffee fund so I can write more like it.