> On Apr 25, 2017, at 9:48 PM, Quincey Morris
> <[email protected]> wrote:
>
> But put that aside. The *real* truth is that MRR memory management has always
> been broken, because we relied on a heuristic for ensuring that memory
> deallocations don’t happen at inopportune times. In particular, even if a
> getter returns an autoreleased reference, you must still retain the returned
> value *if there’s a possibility that the autorelease pool will be drained*
> while the returned value is still in use though un-re-retained. Luckily, this
> was an uncommon scenario.
Not so uncommon, in fact. Here’s a really easy one to get burned by:
> - (BOOL)doSomethingAndReturnError:(NSError **)error {
> NSArray *things = [self getSomeThings];
>
> // Try to do something for each thing. If one of the things fails,
> error out.
>
> for (Thing *eachThing in things) {
> if (![eachThing doSomethingAndReturnError:error]) {
> return NO;
> }
> }
>
> return YES;
> }
This works. But then, it turns out that the app consumes an unreasonable amount
of memory when you have a large number of things. Upon further analysis, it
turns out that -[Thing doSomethingAndReturnError:] generates a lot of
autoreleased objects, and since those never get drained until the loop is done,
it creates a massive memory spike. Uh oh! Well, that’s okay, all we need to do
is an an @autoreleasepool to the loop, right?
> - (BOOL)doSomethingAndReturnError:(NSError **)error {
> NSArray *things = [self getSomeThings];
>
> // Try to do something for each thing. If one of them fails, error out.
>
> for (Thing *eachThing in things) @autoreleasepool {
> if (![eachThing doSomethingAndReturnError:error]) {
> return NO;
> }
> }
>
> return YES;
> }
You build this, send it to the testers, everyone tries it with a bunch of
different collections of things of different sizes, it all seems to work. Ship
that sucker!
And then one of your users finally gets the rare runtime error that -[Thing
doSomethingAndReturnError:] hardly ever throws, and your program crashes,
because the error got reaped by the autorelease pool. D’oh.
This can bite you in ARC, too, because by-reference object parameters are
autoreleasing by convention, to preserve compatible with MRC clients. It’s
tough to work around, too. Look at the above. You’d think you’d just be able to
fix it like this, right?
> - (BOOL)doSomethingAndReturnError:(NSError **)error {
> NSArray *things = @[[Thing new], [Thing new], [Thing new]];
> NSError *theError = nil;
>
> for (Thing *eachThing in things) @autoreleasepool {
> if (![eachThing doSomethingAndReturnError:&theError]) {
> if (error) *error = theError;
> return NO;
> }
> }
>
> return YES;
> }
I’ve got an error variable declared outside the autoreleasepool, I assign to
it, it gets retained, right? Wrong, because our local retain is balanced by a
release when the method returns, and when it gets assigned to ‘error’, it gets
autoreleased—inside our @autoreleasepool block. D’oh. So you’ve either got to
declare the error parameter strong to keep the error out of the autorelease
pool, potentially tripping up any MRC clients and causing them to leak:
> - (BOOL)doSomethingAndReturnError:(NSError * __strong *)error {
> NSArray *things = @[[Thing new], [Thing new], [Thing new]];
> NSError *theError = nil;
>
> for (Thing *eachThing in things) @autoreleasepool {
> if (![eachThing doSomethingAndReturnError:&theError]) {
> if (error) *error = theError;
> return NO;
> }
> }
>
> return YES;
> }
Or, get rid of the early returns and keep a single return value that you don’t
return until the end, which might get unclear and unwieldy if the method is
long:
> - (BOOL)doSomethingAndReturnError:(NSError **)error {
> NSArray *things = @[[Thing new], [Thing new], [Thing new]];
>
> BOOL failed = NO;
> NSError *theError = nil;
>
> for (Thing *eachThing in things) @autoreleasepool {
> if (![eachThing doSomethingAndReturnError:&theError]) {
> failed = YES;
> break;
> }
> }
>
> if (failed && error != NULL) {
> *error = theError;
> }
>
> return !failed;
> }
Or use a goto:
> - (BOOL)doSomethingAndReturnError:(NSError **)error {
> NSArray *things = @[[Thing new], [Thing new], [Thing new]];
> NSError *theError = nil;
>
> @try {
> for (Thing *eachThing in things) @autoreleasepool {
> if (![eachThing doSomethingAndReturnError:&theError]) {
> return NO;
> }
> }
>
> return YES;
> }
> @finally {
> if (error) *error = theError;
> }
> }
None of these are particularly pleasing to the eye, but the most insidious
thing is that it’s dead easy to miss that you need to do this if the method in
question hardly ever throws an error.
Summary: I agree with you that MRC is/was broken. It’s a shame that we’re stuck
with the concept of autorelease in ARC Objective-C, and even in Swift to some
extent. I’m certain that if we’d had ARC from the beginning, autorelease and
its assorted array of problems would never have existed.
Charles
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Objc-language mailing list ([email protected])
Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/objc-language/archive%40mail-archive.com
This email sent to [email protected]
