rubymotion-community / BubbleWrap

Cocoa wrappers and helpers for RubyMotion (Ruby for iOS and OS X) - Making Cocoa APIs more Ruby like, one API at a time. Fork away and send your pull requests
Other
1.18k stars 208 forks source link

Fix KVO that breaks if a target object overrides 'isEqual' and 'hash' #361

Open tutuming opened 10 years ago

tutuming commented 10 years ago

NSDictionary's key is compared by key object's hash. In BW::KVO targets are held as dictionary's key. So if target objects' class override 'hash' (and 'isEqual'), KVO doesn't work collectly.

clayallsopp commented 10 years ago

This makes sense and I believe it shouldn't cause any trouble (tests fail for an unrelated reason, #359).

But I would like to get someone else to look at it - @colinta or @markrickert? I believe it will remove a strong reference to the target, which may have consequences in some apps?

colinta commented 10 years ago

The strong reference to target does make me pause, but it's probably not good to have that strong reference anyway; does that array get reset at some point, or is that a possible memory leak?

I think it should be merged, with a good explanatory email to the group about the change.

colinta commented 10 years ago

So yeah it looks like @targets never gets reset/reassigned, so whatever gets put in there stays in there. We should do some cleanup in unobserve/unobserve_all.

tutuming commented 10 years ago

@clayallsopp @colinta

Thanks for your reviews! I added cleanup code for @targets (and a little refactoring), is that right?

clayallsopp commented 10 years ago

@tutuming can you merge with the master branch again and ensure that the tests pass on travis? thanks!