The only thing that surprised me was the reference cycle in NSAssert. Then again, given Apple's poor regard for TDD, that shouldn't surprise me.
Everyone except the authors of the "Blocks Programming Topics" and the authors of the AVCamCaptureManager sample code. Maybe.
FYI the reason I included the digression about the __block change specifically is that I was recently fixing bugs on a project that uses AVCamCaptureManager, and Apple's mis-use of __block in that class, this is not a joke, through a series of corner cases, caused the status bar to unexpectedly and nondeterminisitically change color about 5% of the time on an unrelated screen.
Saying "everybody should already know how to do weak references" is great in theory. But if you are fielding weird reports for unreproducible status bar issues and it occurs to you at any time in the first hour that maybe Jim from the next cubicle used buggy sample code for a video recording feature on another screen you are a WAY better software developer than I am.
Of course this is standard stuff. But unlike a lot of standard stuff, this one can go undetected for long periods, and crop up in very unexpected places. That's the problem.
No - it's just that these documents/samples haven't been updated when they should have been. Have you filed a Radar?
For me, I agree this is standard stuff. If you are writing this stuff everyday, this block song and dance is basically taken care of with muscle memory.
Part of my job involves working with people from other companies who aren't as strong with objective-c. There are a lot of people that have trouble blocks for whatever reasons. I recently had a guy from another company act almost surprised that we were using "advanced features like blocks". We were a little shocked. But it just shows that not everyone is comfortable with them.
BUT, I think a better approach from the author wouldn't be to tell people not to use blocks w/ NSNotificationCenter but to make sure they know the in and outs of blocks, because blocks are amazingly powerful tools and they aren't going anywhere.
The NSAssert macro caught me by surprise too.
libextobj is great.
So it can introduce reference cycles that only exist in debug mode, that prevent you from spotting premature deallocations that only become apparent in release mode.
(Of course, you QA in release mode. But it's annoying not catching that stuff early)
A hack to get around the self inside the macro https://github.com/seivan/SHAlertViewBlocks/blob/develop/Exa...
The only thing that makes this case confusing is that self only holds the block implicitly (via the notification center instead of any obvious ivar). Otherwise it's a design pattern that every reference counted language user must understand to avoid memory leaks. Weak references to self in blocks are a very common design pattern that all Objective-C programmers should know.
If you access an instance variable by reference, a strong reference is made to self;
If you access an instance variable by value, a strong reference is made to the variable."
Drew also gets bitten by NSAssert using self. I would suggest that it is NSAssert that is doing something dodgy here - it's trying to have an implicit self, as though it was a method, when it most definately isn't a method - and Drew just got bitten by the impedence mismatch. Macros - just say no.
ReactiveCocoa has some nice patterns for avoiding block callbacks altogether. Eg:
[myTarget rac_liftSelector:@selector(receiveCallback:) withSignals:mySignal, nil];
__weak MyClass* wself = self;
self->observer = [[NSNotificationCenter defaultCenter]
addObserverForName:@"heyho"
object:nil queue:nil
usingBlock:^(NSNotification *n) {
MyClass *sself = wself;
if (sself) {
NSLog(@"%@", sself);
}
}];
Also, it's my understanding closures only cause retain cycles if the method taking the block copies the block, per the docs: "When a block is copied, it creates strong references to object variables used within the block." Therefore this dance is not necessary for e.g. [NSURLConnection sendAsynchronousRequest:queue:completionHandler:]. Is that correct?
If you over-do it then NSNotifications easily can become distributed goto. Once I had to add features to a Mac application that made heavy (ab)use of NSNotifications. Following code paths was a nightmare.
I tend to prefer the delegate pattern because there the relationships between objects are clear. Even if it means more work (creating glue code) - your sanity is worth it.
Just a heads up though: The code snippet were hard to read because of the indentation. May I suggest
cleanupObj = [[NSNotificationCenter defaultCenter]
addObserverForName:notificationName
object:nil
queue:nil
usingBlock:^(NSNotification *note) {
// Code!
}];
1. The test is depending on dealloc being done eagerly.
2. The Attempt class puts unsubscribe code in its dealloc method.
3. The Attempt class does not force callers to control its lifetime.
I'll go into more depth on these.
1. Cleanup is often deferred. Dealloc may only run AFTER the test instead of DURING the loop (e.g. I think NSArray defers releasing its items in some cases). Tests that depend on cleanup occurring eagerly, without explicitly waiting or forcing it, are brittle. Given that Attempt uses dealloc to unsubscribe, and we are testing that it unsubscribed, we should at the very least allocate it in an autoreleasepool that ends before we test that it unsubscribed.
2. Putting unsubscribe code in dealloc is a great way to upgrade minor memory leak bugs into major behavior bugs. The article gives fantastic examples of this happening many different ways. Don't rely on the object happening to go out of scope at the right time. If your object's lifetime is controlling whether or not side effects occur, that's a bad situation to be in.
3. Someone has to be responsible for when unsubscribing happens (since we took it away from dealloc), and the someone most in a position to know when is the caller. We should force them to tell us our subscription lifetime. I like to control lifetimes with cancellation tokens [1], so I'd write the attempt class like this:
@implementation Attempt
-(instancetype) initUntil:(TOCCancelToken*)untilCancelledToken {
if (self = [super init]) {
id cleanupObj = ... addObserverForName stuff from Attempt1 ...
[untilCancelledToken whenCancelledDo:^{
[NSNotificationCenter.defaultCenter removeObserver:cleanupObj];
}];
}
return self;
}
- (void)setLocalCounter:(int)localCounter { ... }
- (int)localCounter { ... }
@end
-(void)testExample {
for(int i =0; i < 5; i++) {
TOCCancelTokenSource* c = [TOCCancelTokenSource new];
Attempt *a = [[Attempt alloc] initUntil:c.token];
[NSNotificationCenter.defaultCenter postNotificationName:notificationName object:nil];
[c cancel];
XCTAssertEqual(counter, i+1, @"Unexpected value for counter.");
XCTAssertEqual(1, attempt1.localCounter, @"Unexpected value for localCounter.");
}
}
1: http://twistedoakstudios.com/blog/Post7391_cancellation-toke...
I dislike that receiving the notification mutates global state though. I can't clearly articulate it, but that's why I think the caller should have explicit control over when the behavior is active. I think notification subscriptions that last for the lifetime of the object should probably be related to the object's internal state.
Finally, I went to the clang docs regarding timing of dealloc, and as I read it, they shoot for immediate deallocation, but let it slide for local variables:
Precise lifetime semantics
In general, ARC maintains an invariant that a retainable object pointer
held in a __strong object will be retained for the full formal lifetime
of the object. Objects subject to this invariant have precise
lifetime semantics.
By default, local variables of automatic storage duration do
not have precise lifetime semantics. Such objects are simply strong
references which hold values of retainable object pointer type,
and these values are still fully subject to the optimizations
on values under local control.
Rationale
Applying these precise-lifetime semantics strictly would be prohibitive.
Many useful optimizations that might theoretically decrease the
lifetime of an object would be rendered impossible. Essentially,
it promises too much.
A local variable of retainable object owner type and automatic storage
duration may be annotated with the objc_precise_lifetime attribute to
indicate that it should be considered to be an object with precise lifetime
semantics.
Rationale
Nonetheless, it is sometimes useful to be able to force an object to
be released at a precise time, even if that object does not appear
to be used. This is likely to be uncommon enough that the syntactic
weight of explicitly requesting these semantics will not be burdensome,
and may even make the code clearer.self.block = ^{ typeof(self) strongSelf = weakSelf; if (strongSelf) { // ... } };
It lives only in the 'platform' layer of my code. Grudgingly because Apple enforce it.
It could be worse... they could have pulled a Google and used Java - then held back the tools they develop e.g. the Java VM because they are 'dangerous' and suggest that using native code is 'bad' if it is just for performance or cross-platform reasons.
At least the interoperability with sane programming languages in Objective-C is excellent.
I'd point out the vast majority of memory management issues I have is when using someone's refcounting or gc scheme. new and delete are exactly what i want all of the time. i like to tell the machine what to do and i have developed non-trivial software without leaks /before testing for leaks/ because once you have some practice with new and delete it becomes easy and you will never look back...
As a mechanism for broadcasting information throughout an app NSNotificationCenter is slower, more complicated and (apparently) more dangerous than my hand rolled code. That should be shocking and its counter to the common idea that 3rd party and especially OS libraries should be better... there are a number of cases where they measurably aren't.
(The overhead of the objective C message mechanism - or even the RTTI mechanism which is a small part of that, exceeds that of adding or reading something from a well implemented threadsafe data structure (which is the first step to either of these things in most cases))
Objective-C is measurably bad for my code. I have reams of data to support this... and its all trivially reproducable.
Whatever you intended, it came across as a bit of a rant.
i also don't like the ref counting. when given a choice i would rather do my own allocations.