NSNotificationCenter is thread-safe NOT

April 21, 2014

According to Apple's Threading Programming Guide, NSNotificationCenter is a thread-safe class. "You can use the same instance from multiple threads without first acquiring a lock." However, this is extremely misleading. In fact, if your app has multiple threads, then you're almost certainly using NSNotificationCenter wrong. Almost everyone makes the same mistake:

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

To see why this is wrong, consider the following example.

@interface MyObserved : NSObject
@end

@implementation MyObserved

-(void)postNotification
{
	[[NSNotificationCenter defaultCenter] postNotificationName:@"MyObservedNotification" object:self userInfo:nil];
}

-(id)init
{
	self = [super init];
	if ( self != nil )
	{
		[self performSelectorInBackground:@selector( postNotification ) withObject:nil];
	}
	return self;
}

@end

@interface MyObserver: NSObject
{
	MyObserved *_myObserved;
}
@end

@implementation MyObserver

-(void)observedNotification:(NSNotification *)notification
{
	NSLog( @"begin %@", self );
	sleep( 1 );
	NSLog( @"end %@", self );
}

-(id)init
{
	self = [super init];
	if ( self != nil )
	{
		_myObserved = [[MyObserved alloc] init];
		[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector( observedNotification: ) name:@"MyObservedNotification" object:_myObserved];
	}
	return self;
}

-(void)dealloc
{
	[[NSNotificationCenter defaultCenter] removeObserver:self];
	NSLog( @"dealloc %@", self );
	[_myObserved release];
	[super dealloc];
}

@end

@implementation AppDelegate

-(void)applicationDidFinishLaunching:(NSNotification *)notification
{
	MyObserver *myObserver = [[[MyObserver alloc] init] autorelease];
	NSLog( @"myObserver %@", myObserver );
}

@end

The example is contrived, to be sure, but that's only to promote reproducibility and illustrate the problem. The above code crashes reliably in the NSLog after sleep. Why? What we see here is that removeObserver: does not block until all notifications have been posted. The method can return while a notification is still executing on another thread. Thus, we have a race condition. I intentionally extended the execution time of the notification by a full second, but if we get down to the level of milliseconds, then we can end up with random bugs, which is even worse than reproducible bugs, because they're a lot harder to debug.

What are the lessons to be learned from our example?

  1. Only post a particular notification on one thread. Ideally that would be the main thread, but in any case, be consistent.
  2. It is only safe to add and remove a notification observer on the same thread that posts the notification.
  3. In a multi-threaded app, it's difficult to guarantee which thread will call dealloc.
  4. Never remove an observer in dealloc.
  5. This is what is sounds like when doves cry.