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

Multi-instance Memory Leak #939

Closed
gregberns opened this issue Jan 6, 2017 · 16 comments
Closed

Multi-instance Memory Leak #939

gregberns opened this issue Jan 6, 2017 · 16 comments
Labels
bug Something isn't working

Comments

@gregberns
Copy link

gregberns commented Jan 6, 2017

On Android, when using react-native-maps with the react-native-router-flux router, when a map is loaded over and over, there is a large memory leak that can result in an application crash.

It appears that the map instance is not disposed of. After a short look at a heap dump, it seems like 'AirMapView' is not getting disposed. I suspect this is a native Java wrapper around the Google Maps instance.

It looks like the issue was introduced after 0.10.1, and exists in 0.11.0.

I've created a simple project to make this issue easy to reproduce.

https://github.com/gregberns/ReactNativeMapTest

I haven't done much work with this project or RN's native components, but I'll try and dig in a little further and see where it can go, but any help to resolve this would be very welcome.

@gregberns
Copy link
Author

Looks this was introduced by PR #694, made by @felipecsl , merged by @spikebrehm. Calls to AirMapView.doDestroy() were removed to fix #414. Thats the only change I can see that seems like it could cause the extra map instances.

@felipecsl
Copy link
Contributor

Thanks for reporting this. Sounds like a legit issue. We'll take a look!

@felipecsl felipecsl added the bug Something isn't working label Jan 6, 2017
@proProbe
Copy link

how is it going with this bug? Im currently running into this problem!

@felipecsl
Copy link
Contributor

no progress yet!

@gregberns
Copy link
Author

Anything we can do to help? Is it a pretty deep issue? Any details you can share with the underlying issue? Thx

@aashna208
Copy link

On debugging I found two causes for this memory leak:

  1. The google maps api holds the instance to the context if the location is enabled. Here is the tracking issue: https://code.google.com/p/gmaps-api-issues/issues/detail?id=9233#makechanges. This seems to be happening even in version 9.8.0
  2. AirMapManager is leaking because we are keeping the ThemedReactContext reference. I have a fix for this one.

@mattshen
Copy link
Contributor

mattshen commented Mar 10, 2017

@aashna208 can you post the fix here for point 2 you've mentioned. I would like to give it a try.

mattshen referenced this issue in phantomlds/react-native-maps Mar 13, 2017
mattshen referenced this issue in phantomlds/react-native-maps Mar 13, 2017
rops pushed a commit to tt-sport-mobile/react-native-maps that referenced this issue Mar 13, 2017
@mattshen
Copy link
Contributor

mattshen commented Mar 15, 2017

Just created a PR (#1130). It reverts some changes in PR#694, and puts some extra patch from here (IjzerenHein@14ca6f7).

@psam44
Copy link

psam44 commented Mar 19, 2017

@mattshen I had a look at your PR. Without knowing it, in my issue 1102 I came to a similar conclusion, with a fix fully in AirMapView.java (no change in AirMapManager.java). (I don't know if one is better than the other).

@mattshen
Copy link
Contributor

Hi @psam44, Just saw you've updated issue#1102 with more details. Will try that out later

@lelandrichardson
Copy link
Collaborator

Closed - should be fixed by #1130 thanks @mattshen!

@IjzerenHein
Copy link
Contributor

Has anyone tested this yet? I still seem to be loosing memory when showing and hiding MapView.
Will try to investigate.

@freeljt
Copy link

freeljt commented May 2, 2017

With react-native-maps: "0.14.0", I still see memory jumping by ~100Mb when rendering by looking at perf monitor in iOS simulator. And the memory usage did not fully recover after the map component unmount.
Is there anyway to reduce the ram usage or recover from the previous rendering?

@mattshen
Copy link
Contributor

mattshen commented May 2, 2017

same on Android, the memory isn't fully recovered after unmounting the map view. I guess it's somehow related to GC. However, we haven't experienced any crashes or serious issues regarding this.

@austin43
Copy link

austin43 commented Jun 6, 2017

Experiencing the same problem on iOS simulator @freeljt
Before mounting ~10 different maps within the span of a minute:
https://www.dropbox.com/s/qc3lo8e6s796maf/Screenshot%202017-06-06%2016.05.11.png?dl=0

After:
https://www.dropbox.com/s/vavli8gcaxkka6d/Screenshot%202017-06-06%2016.06.06.png?dl=0

It drops back down after a while from what I assume is RN's GC, but not sure of an interim fix.

@j-mendez
Copy link

memory issues here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests