Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Views are not unsubscribed from NSNotificationCenter #12765

Closed
tido64 opened this issue Mar 7, 2017 · 1 comment
Closed

Views are not unsubscribed from NSNotificationCenter #12765

tido64 opened this issue Mar 7, 2017 · 1 comment
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@tido64
Copy link
Collaborator

tido64 commented Mar 7, 2017

Description

We have a hybrid app that uses React Native for a single view. When we navigate from it, there are some bytes that are not freed. On closer inspection, I noticed that some classes call [NSNotificationCenter addObserver:…] in init and [NSNotificationCenter removeObserver:…] in dealloc. With ARC enabled, before iOS 9.0, NSNotificationCenter will retain a strong reference so dealloc will never be called, causing a leak. See here for more details.

This accounts for some of the leaks we're currently seeing in our app but not all so we're still looking into it. Maybe this accounts for some of the leaks mentioned in #11961.

Solution

There are several solutions here:

  • Bump all deployment targets to iOS 9.0.
  • Only pass a weak reference to [NSNotificationCenter addObserver:…].
  • Move the call to [NSNotificationCenter removeObserver:…] out of dealloc to somewhere more appropriate, like viewWillDisappear: or equivalent.

I'm not sure which solution is more preferred here (hence the lack of a PR) but if you still want to support iOS 8.0, then I'd propose solution 2 as the simplest to implement:

diff --git a/Libraries/Image/RCTImageView.m b/Libraries/Image/RCTImageView.m
index 6387987e6..0cbdc8ca1 100644
--- a/Libraries/Image/RCTImageView.m
+++ b/Libraries/Image/RCTImageView.m
@@ -87,12 +87,13 @@ static NSDictionary *onLoadParamsForSource(RCTImageSource *source)
   if ((self = [super init])) {
     _bridge = bridge;

+    __weak RCTImageView *weakSelf = self;
     NSNotificationCenter *center = [NSNotificationCenter defaultCenter];
-    [center addObserver:self
+    [center addObserver:weakSelf
                selector:@selector(clearImageIfDetached)
                    name:UIApplicationDidReceiveMemoryWarningNotification
                  object:nil];
-    [center addObserver:self
+    [center addObserver:weakSelf
                selector:@selector(clearImageIfDetached)
                    name:UIApplicationDidEnterBackgroundNotification
                  object:nil];

Additional Information

  • React Native version: 0.41
  • Platform: iOS
  • Operating System: macOS
@javache
Copy link
Member

javache commented Mar 28, 2017

With ARC enabled, before iOS 9.0, NSNotificationCenter will retain a strong reference

That's not true. Before iOS9, NSNotificationCenter holds an unsafe-unretained reference to your view, from iOS10 on it's a weak reference instead. Neither of these increase the retain count of your objects and don't prevent deallocation.

Passing a weak reference to NSNotificationCenter also doesn't work that way, since you're effectively turning it into a strong reference when using it as an argument.

@javache javache closed this as completed Mar 28, 2017
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

3 participants