iOS: KVO Memory Leak?

Question

Note: I am posting this as a reference for other developers that might run into the same issue.

Why do I have a memory leak with this code:

@interface SPWKThing : NSObject
@property (strong, nonatomic) NSArray *things;
@end

@implementation SPWKThing {
    BOOL _isKVORegistered;
}

- (id)init
{
    self = [super init];
    if (self) {
        NSLog(@"initing SPWKThing");
        [self registerKVO];
    }
    return self;
}

- (void)didChangeValueForKey:(NSString *)key {
    if ([key isEqualToString:@"things"]) {
        NSLog(@"didChangeValueForKey: things have changed!");
    }
}

#pragma mark - KVO
- (void)registerKVO
{
    if (!_isKVORegistered) {
        NSLog(@"Registering KVO, and things is %@", _things);
        [self addObserver:self forKeyPath:@"things" options:0 context:NULL];
        _isKVORegistered = YES;
    }
}

- (void)unregisterKVO
{
    if (_isKVORegistered) {
        NSLog(@"Unregistering KVO");
        [self removeObserver:self forKeyPath:@"things"];
        _isKVORegistered = NO;
    }
}

- (void)dealloc
{
    NSLog(@"SPWKThing dealloc");
    [self unregisterKVO];
}

@end

@implementation SPWKViewController

- (void)viewDidLoad
{
    [super viewDidLoad];
    [self runDemo];
}

- (void)runDemo
{
    SPWKThing *thing = [[SPWKThing alloc] init];
    thing.things = @[@"one", @"two", @"three"];
    thing = nil;
}

@end

My output is:

initing SPWKThing
Registering KVO, and things is (null)
didChangeValueForKey: things have changed!

dealloc is never called? Why? I am setting thing = nil in the last line of runDemo!

See a demo project here: https://github.com/jfahrenkrug/KVOMemoryLeak

Why in the world are you listening to a change on yourself? Just put whatever code that reacts to the change in `setThings` somewhere...unless this question is an exercise for discussion... — Tim Reddy, Mar 03 '14 at 20:47
So what happens if you put a breakpoint in `unregisterKVO`? Your use of `BOOL` in your implementation is something I've never done before...usually I use a class extension in the `.m` file to put private iVars there... (i.e., `@interface SPWKThing() {BOOL _isKVORegistered;} @end` — Tim Reddy, Mar 03 '14 at 20:50
@TimReddy This is "an exercise for discussion" :) Basically I'm sharing my discovery that overriding `didChangeValueForKey:` without calling `super` causes a memory leak. About the `BOOL`: http://stackoverflow.com/a/8853683/171933 — Johannes Fahrenkrug, Mar 03 '14 at 20:55
What happens if you remove all the KVO code? Does your `dealloc` get called? — Merlevede, Mar 03 '14 at 21:06
@Merlevede Yes. If `addObserver:forKeyPath:options:context:` is never called, then the object get correctly dealloced. — Johannes Fahrenkrug, Mar 03 '14 at 21:15

score 3 · Accepted Answer · answered Mar 03 '14 at 20:43

The answer is:

Never override didChangeValueForKey: (at least not without calling super). The documentation does not warn you about this.

Use the correct method observeValueForKeyPath:ofObject:change:context: instead.

This project clearly demonstrates this: https://github.com/jfahrenkrug/KVOMemoryLeak

iOS: KVO Memory Leak?

1 Answers1