Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Retain Cycle Fixes #386

Merged
merged 6 commits into from
Feb 16, 2016
Merged

Retain Cycle Fixes #386

merged 6 commits into from
Feb 16, 2016

Conversation

dzenbot
Copy link

@dzenbot dzenbot commented Feb 10, 2016

For some yet unknown reasons, registering UIKeyCommand objects from within SLKTextViewController caused the view controller never to be deallocated, when interacting with at least 1 of these commands on the iPad. Certainly due to a retain cycle since this didn't cause any memory leaks. With the right repro steps, it was possible to get incremental retain cycles.

After profiling this for a full day, and looking for different alternatives, I decided to move ALL the key command registration back to SLKTextView, using a simpler-to-use API like:

- (void)observeKeyInput:(NSString *)input
              modifiers:(UIKeyModifierFlags)modifiers
                  title:(NSString *)title
             completion:(void (^)(UIKeyCommand *keyCommand))completion;

Key commands are then registered internally in SLKTextView, using a common selector and forwarding the events to the block callbacks. By doing this, the view controller is out of risk from being retained, possibly by a UIKit private object since it was untraceable. It seems to me, that this is a behavior change on UIKit since iOS 9, but unsure.

Ignacio Romero Zurbuchen added 2 commits February 10, 2016 12:11
@dzenbot dzenbot added the bug label Feb 10, 2016
@dzenbot dzenbot added this to the v2 milestone Feb 12, 2016
Ignacio Romero Zurbuchen added 2 commits February 13, 2016 02:55
… a totally new approach for better key input observation using completion blocks.

This finally fixes a strange retain cycle caused by interacting with key commands in SLKTextViewController.
@dzenbot dzenbot removed this from the v2 milestone Feb 13, 2016
@sukeban
Copy link
Contributor

sukeban commented Feb 16, 2016

This code looks good. Such an annoying bug!

sukeban added a commit that referenced this pull request Feb 16, 2016
@sukeban sukeban merged commit 30f3ecc into master Feb 16, 2016
@dzenbot dzenbot deleted the retain-cycle-fix branch February 16, 2016 21:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants