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

Memory leak in iOS #92

Closed
xutm opened this issue Apr 29, 2019 · 5 comments
Closed

Memory leak in iOS #92

xutm opened this issue Apr 29, 2019 · 5 comments

Comments

@xutm
Copy link

xutm commented Apr 29, 2019

bug

Used by in iOS will cause memory leak.

image

image

env

react-native: 0.59.4
react: 16.8.3
react-navigation: 3.5.1
react-native-screens: 1.0.0-alpha.22

demo

import React from "react";
import { View, Text } from "react-native";
import { createStackNavigator, createAppContainer } from "react-navigation";
import { useScreens } from 'react-native-screens';
useScreens();

class HomeScreen extends React.Component {
  render() {
    return (
      <View style={{ flex: 1, alignItems: "center", justifyContent: "center" }}>
        <Text>Home Screen</Text>
      </View>
    );
  }
}

const AppNavigator = createStackNavigator({
  Home: {
    screen: HomeScreen
  }
});

const AppContainer = createAppContainer(AppNavigator)

AppRegistry.registerComponent('AwesomeProject', () => AppContainer)
@ericlewis
Copy link

@xutm what was your methodology here?

@zhigang1992
Copy link

From the initial investigation, even though

https://github.com/kmagiera/react-native-screens/blob/master/ios/RNSScreen.m#L59

That view weak referencing controller,

But with

https://github.com/kmagiera/react-native-screens/blob/master/ios/RNSScreen.m#L103

It became a strong reference again.

@kmagiera
Copy link
Member

Hey @xutm and @zhigang1992

One note is that the fact retain cycle exists does not imply there is a memory leak. The retain cycle you've found here is due to the fact RNSScreenView object retains RNSScreen which inherits UIViewController and then loads RNSScreenView as its content. UIViewController require that the loaded view is retained. Note that the cycle only exists while the view is loaded into a controller and we break the cycle here https://github.com/kmagiera/react-native-screens/blob/master/ios/RNSScreen.m#L47 when the view is detached from RN hierarchy. In general we want RNSScreen (which inherits UIViewController) to exists so long RNSScreenView exists. Therefore the way it is currently handled seem correct to me. To be clear the way means that we break the cycle when view is detached from hierarchy.

I could be wrong and maybe the case you are describing actually causes a leak but your app does not seem to reproduce it. In your app you load a single screen, there is nothing that can leak there because the screen needs to be visible, we can't free the memory. Therefore I'm assuming you are only talking about the existence of retain cycle not about memory leak.

osdnk pushed a commit that referenced this issue Jan 11, 2020
This issue is related to #92.

Issue
I got issue when playing video with react-native-video. The video keep playing (still hear sound) when go back.

react-native: 0.61.5
react-native-screens: 2.0.0-alpha.22
Solution
When controller popped, try to assign view to nil to break the cycle.
@WoLewicki
Copy link
Member

This issue seems outdated. Can I close it @xutm ?

@WoLewicki
Copy link
Member

I am closing it due to no response in more than 30 days. Feel free to comment to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants