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

Fix Memory Leaks on iOS #2716

Merged
merged 2 commits into from Feb 27, 2019

Conversation

Projects
None yet
3 participants
@kuglemic
Copy link
Contributor

kuglemic commented Feb 27, 2019

This PR fixes a memory leak on iOS using Apple Maps where AIRMap didn't get deallocated if the component is unmounted during an regionChange animation.

Does any other open PR do the same thing?

No

What issue is this PR fixing?

Didn't file an issue for it and haven't found one.

How did you test this PR?

I did test it on the iPhone 8 Simulator and on a real iPhone 7 device.

Steps to reproduce:

  1. Create a simple Test App where you can mount and unmount the Map Component.
  2. Mount the Map component
  3. Change the region of the map animated, e.g. by calling: animateToRegion
  4. Make sure that - (void)mapView:(AIRMap *)mapView regionWillChangeAnimated:(__unused BOOL)animated gets called. This method did cause the leak.
  5. Unmount the Map component during the animation, this means before - (void)mapView:(AIRMap *)mapView regionDidChangeAnimated:(__unused BOOL)animated did get called.
  6. Notice that dealloc didn't get called in AIRMap.m
  7. If you Mount the Map component again, you will get a new instance of AIRMap, which causes a memory increase of ~30MB in my simulator

kuglemic added some commits Feb 27, 2019

@alvelig

This comment has been minimized.

Copy link
Collaborator

alvelig commented Feb 27, 2019

These are the PR we are missing. Thank you. LGTM @rborn 🐽

@rborn rborn merged commit f002785 into react-native-community:master Feb 27, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.