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.

Sign up for GitHub

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

Jump to bottom

[iOS] Memory leak in RCTModalHost? #9265

Closed
rigdern opened this issue Aug 6, 2016 · 2 comments
Closed

[iOS] Memory leak in RCTModalHost? #9265

rigdern opened this issue Aug 6, 2016 · 2 comments
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.

Comments

@rigdern
Copy link
Contributor

rigdern commented Aug 6, 2016

It looks like RN currently isn't calling invalidate on any of the RCTModalHostViews because _hostViews is never allocated. I think this line should have a ! in the condition.

My assumption is that the intention of this line is to allocate _hostViews exactly 1 time but it looks like it's currently allocating it only if it is already allocated.
@ide
Copy link
Contributor

ide commented Aug 7, 2016

That's almost certainly a bug and runs only because of Obj-C's safe nil calls. Would anyone like to send a quick PR?

@ide ide added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: iOS iOS applications. Help Wanted :octocat: Issues ideal for external contributors. labels Aug 7, 2016
@Roshanjossey Roshanjossey mentioned this issue Aug 8, 2016
Closed
ghost pushed a commit that referenced this issue Aug 8, 2016
Fix memory leak bug in RCTModalHost
915345b 
Summary:
This PR addresses issue #9265.

Negated the condition that checks if `_hostViews` exists so that `_hostViews` will be allocated only if it doesn't exist
Closes #9295

Differential Revision: D3686214

fbshipit-source-id: ec0caac99d231786eefad023f3f0ed44a79f687e
@Roshanjossey
Copy link

@ide: Can we close this issue?

@ide ide closed this as completed Aug 10, 2016
cmcewen pushed a commit to cmcewen/react-native that referenced this issue Aug 15, 2016
@cmcewen
Fix memory leak bug in RCTModalHost
898d41c 
Summary:
This PR addresses issue facebook#9265.

Negated the condition that checks if `_hostViews` exists so that `_hostViews` will be allocated only if it doesn't exist
Closes facebook#9295

Differential Revision: D3686214

fbshipit-source-id: ec0caac99d231786eefad023f3f0ed44a79f687e
mpretty-cyro pushed a commit to HomePass/react-native that referenced this issue Aug 25, 2016
Fix memory leak bug in RCTModalHost
d78f6d8 
Summary:
This PR addresses issue facebook#9265.

Negated the condition that checks if `_hostViews` exists so that `_hostViews` will be allocated only if it doesn't exist
Closes facebook#9295

Differential Revision: D3686214

fbshipit-source-id: ec0caac99d231786eefad023f3f0ed44a79f687e
@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 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rigdern @ide @Roshanjossey @react-native-bot