-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fixes google map null pointer exception #1403
Fixes google map null pointer exception #1403
Conversation
AirMapView doDestroy is called before LifecycleEventListener onHostPause. This fixes react-native-maps#1358
Do you have test phone that instantly crashes without this PR? So you can confirm that this works :) |
@henrikra This was crashing on my simulator with the same Nullpointer exception, some debugging led me to the bug. If you have access to a device to reproduce this issue then you can test it. It is a small change |
Unfortunately I don't have phone to reproduce this. But we definately need one |
This was actually a problem with my tablet (SAMSUNG T560 Android 4.4.4). |
@IgorVanian I dont think it has anything to do with onPause. This PR fixes a bug where sometimes AirMapView doDestroy (where MapView onDestroy is called) is called before the lifecycle onHostPause (where MapView onPause is called.). So AirMapView was trying to pause a MapView that is already destroyed, hence the NPE |
@chaitanya0bhagvan I think you need to get real device with this problem to really test this out. |
It works perfect on smartphones (it worked even without this PR). On my tablet it freezes when I tap on a marker to open my custom callout view (works on smartphones), but as I said, it fixed the original problem so maybe it's not related, or is it? :D |
@henrikra I dont have access to any device to test this out. I guess since @IgorVanian has the device he might be able to test this out. @IgorVanian Can u write a simple react native app to reproduce this issue and then apply the fix to verify this PR fixes it ? |
Sure. Will do. |
@chaitanya0bhagvan Btw have you noticed that this #1273 tries to solve same problem as you? |
Cannot reproduce with a clean react-native init and vanilla react-native-maps (without this PR). |
@chaitanya0bhagvan Good news! We got actual phone to test which is LG G4. That device crashed instantly before. With this PR is does not crash and has been running on device without problems for 45min now! We are going to try this in production this week. I will tell the results later :) |
@henrikra Good to hear. Glad I could be of help. @IgorVanian I have noticed that this is an intermittent issue, probably when the JS code is busy this is happening. |
@felipecsl Please check this PR. We are so close fixing this annoying issue |
@chaitanya0bhagvan This pr is now in production. I will report if it helps our users not to crash. So far no crashes ;) |
@henrikra Nice 👍 |
This works as a palliative fix but we'll need to fix the underlying problem since we shouldn't have to do this in the first place |
Do you know what is the underlying problem? Btw full day without crashes in production for now :) |
2 days in production with this PR and now we got first crash. It is not exactly the same but very similar: (Xiaomi Redmi 4A)
Any idea why this is happening? |
@henrikra It is crashing inside MapView.onCreate. Not sure what is happening. |
@henrikra I don't have any context as to what Felipe is talking about. I just started using react-native-maps and the very first time I tried it on the android simulator it crashed. I will probably try to understand what's going on if time permits. |
sorry for the vague comment. what I mean is that the very fact that we have to manage lifecycle events manually (by keeping a |
@felipecsl Could different Android operating systems call lifecycle methods in wrong order or something? Good to hear you are getting started with this challenge :) |
@henrikra i dont think so, no. that's very unlikely. |
AirMapView doDestroy is called before LifecycleEventListener onHostPause. This fixes react-native-maps#1358
AirMapView doDestroy is called before LifecycleEventListener onHostPause. This fixes react-native-maps#1358
Don't work for me |
…-upstream * upstream/master: Add minZoom and maxZoom properties for android and ios (react-native-maps#1360) Reference install solution in issue react-native-maps#718 in install docs (react-native-maps#1448) updates npm cache clean command (react-native-maps#1450) v0.15.3 Added BatchedBridge Upgraded ios deps Use prop-types and add supprort for RN 0.45 Allow react 16.0.0-alpha [Android] Code cleanup step I - reformatting (react-native-maps#1415) Fixes google map null pointer exception (react-native-maps#1403) [iOS - Google Maps] Fix animateToCoordinate and animateToRegion (react-native-maps#1115) Update from View.propTypes to ViewPropTypes to match RN v0.44.0 (react-native-maps#1323) Fix import header for React Native 0.44.2 (react-native-maps#1362) Fix a couple typos (react-native-maps#1375)
…nother lifecycle method, as proposed by @Nelrohd.
* 'master' of github.com:Vizzuality/react-native-maps: fix: error syntax on AirMaps max and min zoom level not nil check Add minZoom and maxZoom properties for android and ios (react-native-maps#1360) Reference install solution in issue react-native-maps#718 in install docs (react-native-maps#1448) updates npm cache clean command (react-native-maps#1450) v0.15.3 Added BatchedBridge Upgraded ios deps Use prop-types and add supprort for RN 0.45 Allow react 16.0.0-alpha [Android] Code cleanup step I - reformatting (react-native-maps#1415) Fixes google map null pointer exception (react-native-maps#1403) [iOS - Google Maps] Fix animateToCoordinate and animateToRegion (react-native-maps#1115) Update from View.propTypes to ViewPropTypes to match RN v0.44.0 (react-native-maps#1323) Fix import header for React Native 0.44.2 (react-native-maps#1362) Fix a couple typos (react-native-maps#1375)
…nother lifecycle method, as proposed by @Nelrohd. (react-native-maps#1464)
* 'master' of https://github.com/airbnb/react-native-maps: v0.16.2 Revert "Issue1176 improve ios marker performance by X100 (react-native-maps#1187)" Fix initial region android (react-native-maps#1563) v0.16.1 Enhance Podfile. (react-native-maps#1252) Update marker component (react-native-maps#1428) Add legalNotice constant (react-native-maps#1458) Issue1176 improve ios marker performance by X100 (react-native-maps#1187) Fix initial region native prop (react-native-maps#1546) fix `Archive` configuration for iOS builds (react-native-maps#1550) v0.16.0 Document MapView min/max zoom properties (react-native-maps#1538) Fix timing function used in AnimatedRegion.spring (react-native-maps#1479) Fix crashing the application when a user presses on the map and the Google Play Services need to be updated or at the moment of the process of updating (react-native-maps#1469) skip region monitoring if map object is null (react-native-maps#1443) Zoom level fixes (react-native-maps#1485) Attempt to fix crashes. A variant of react-native-maps#1403 but for another lifecycle method, as proposed by @Nelrohd. (react-native-maps#1464) Handle Android RN 0.47 breaking change (react-native-maps#1481) add MKTileOverlayRenderer (react-native-maps#1357) Add onMapReady callback (react-native-maps#1369)
AirMapView doDestroy is called before LifecycleEventListener onHostPause. This fixes react-native-maps#1358
…nother lifecycle method, as proposed by @Nelrohd. (react-native-maps#1464)
Sometimes AirMapView doDestroy is called before LifecycleEventListener onHostPause. This fixes #1358