KVO blocks API causing KVO crashes on object dealloc

[hartbit]

First of all, thanks so much for BlocksKit. It's a real time saver!

Now the problem: I've started using the KVO APIs (bk_addObserverForKeyPath:task:) and I'm getting some crashes when my objects are being dealloced:

An instance 0xbbf42b0 of class MyClass was deallocated while key value observers were still registered with it. Observation info was leaked, and may even become mistakenly attached to some other object. Set a breakpoint on NSKVODeallocateBreak to stop here in the debugger. Here's the current observation info:

I've spent some time trying to debug this. I'm fairly sure I got rid of all memory issues caused by reference cycles in blocks by doing the weakSelf/strongSelf dance. But this one is strange. The only observers on the objects in question are created by the objects themselves:

__weak id weakSelf = self;

[self bk_addObserverForKeyPath:@"property" task:^(id target) {
    MyClass* strongSelf = self;
    [strongSelf doSomething];
}];

Any ideas?

[zwaldowski]

If it is as written, MyClass* strongSelf = self; is a leak.

[hartbit]

I'm actually using libextobjc:

@weakify(self);

[self bk_addObserverForKeyPath:@"property" task:^(id target) {
    @strongify(self);
    [self doSomething];
}];

But why do you say it's a leak?

[zwaldowski]

Then in that case it's not a leak, heh heh.

[zwaldowski]

What are you deploying to? 5, 6, or 7? Going to try and reproduce.

[hartbit]

7

[hartbit]

Thanks for having a look into it! I tried to point my Podfile to the next branch, but I'm encountering issues: I got a:

unknown option character `W' in: -Wl,-no_compact_unwind`

I remember reading about this before. So I temporarily removed this flag, and now I get more serious link errors:

Undefined symbols for architecture i386:
  "_ffi_mini_call", referenced from:
      -[A2BlockInvocation invoke] in libArrowWordsModel.a(A2BlockInvocation.o)
  "_ffi_mini_prep_cif_machdep", referenced from:
      _ffi_mini_prep_cif_core in libArrowWordsModel.a(prep_cif.o)
ld: symbol(s) not found for architecture i386

[hartbit]

Even if I couldn't use it for my project, I downloaded BlocksKit with your latest commit and managed to write a failing test:

- (void)testRemoveOnDealloc {
    __weak SubjectKVCAndKVO* weakSubject;

    {
        SubjectKVCAndKVO* tempSubject = [SubjectKVCAndKVO new];
        weakSubject = tempSubject;
        [tempSubject bk_addObserverForKeyPath:@"number" task:^(id target) {
            NSLog(@"number: %@", weakSubject.number);
        }];
    }

    XCTAssertNil(weakSubject);
}

To make 100% clear that adding the observer is what keeps the object alive, here is a passing test:

- (void)testRemoveOnDealloc {
    __weak SubjectKVCAndKVO* weakSubject;

    {
        SubjectKVCAndKVO* tempSubject = [SubjectKVCAndKVO new];
        weakSubject = tempSubject;
    }

    XCTAssertNil(weakSubject);
}

If I understand the way the block methods were implemented, the observers are added to a NSMapTable that keeps a weak link to the observed object, so I don't understand what it's failing Perhaps you can help me understand.

[hartbit]

Ok, perhaps I'm going crazy, but I could have sworn that the NSMapTable was previously created with strongToStrongObjectsMapTable. Did this change recently. Can help me understand the implications?

UPDATE: I saw that it changed on last commit. Still trying to understand this.

[hartbit]

Ok I think I get it now. Please correct me if I'm wrong: you now swizzle a new dealloc method on all observed objects. The swizzled dealloc method removes all observers. But as the NSMapTable has strong references to both the observed object and it's observer blocks, it will never get dealloc-ed until the blocks are removed. So what's the point of the swizzled dealloc method? Shouldn't the NSMapTable still hold weak references to the observed objects?

[hartbit]

I changed the NSMapTable back to weakToStrongObjectsMapTable and now the test passes, but the observers are not automatically unregistered:

An instance 0x1031214b0 of class SubjectKVCAndKVO was deallocated while key value observers were still registered with it. Observation info was leaked, and may even become mistakenly attached to some other object.

I then placed a breakpoint in the swizzled dealloc block-method. It gets called all right, but the issue seems to be coming from bk_removeAllBlockObservers: bk_observerBlocks returns nil. The NSMapTable already released the connected observer blocks by then. But in that case, why isn't the dealloc method of _BKObserver not getting called (placed a breakpoint)?

[zwaldowski]

Ah, I see. The map table implementation hasn't been really used before (due to the 5.0 deployment target). The way we store the observers currently is poorly suited for map tables, as the apparently weak keys get purged at some point before dealloc. When we drop iOS 5 (soon), we'll reformat the observer extensions for it.

Currently working on fixing the next branch podspec, thanks for letting me know about it.

[hartbit]

Is there something I can do to use the observer APIs meanwhile without any memory leaks/crashes? Disable the map tables implementation? I'm sorry to bother you again, but I'm in a rush to finish a client project and I've been using these APIs everywhere ^^

[zwaldowski]

Apologies! Using he current "master", just change HAS_MAP_TABLE to 0.

[hartbit]

Saw the changes you made in next to fix the podspec and to disable map tables. Thanks :)

[vjpr]

These changes on next fixed the same issue for me.

[hartbit]

Is it me or has all BlocksKit code disappeared from next?