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

fix: added pending callbacks mechanism to solve npe crash #332

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: added pending callbacks mechanism to solve npe crash #332

wants to merge 1 commit into from

Conversation

shubhamguptadream11
Copy link

@shubhamguptadream11 shubhamguptadream11 commented Sep 4, 2024 •
edited
Loading

Overview

Solves this issue:

Crash Log

Fatal Exception: java.lang.NullPointerException: Listener must not be null
at com.google.android.gms.common.internal.Preconditions.checkNotNull(com.google.android.gms:play-services-basement@@18.1.0:2)
at com.google.android.gms.common.api.internal.ListenerHolders.createListenerKey(com.google.android.gms:play-services-base@@18.0.1:1)
at com.google.android.gms.location.FusedLocationProviderClient.removeLocationUpdates(com.google.android.gms:play-services-location@@20.0.0:6)
at com.reactnativecommunity.geolocation.PlayServicesLocationManager$1.onLocationResult(PlayServicesLocationManager.java:74)
at com.google.android.gms.internal.location.zzaw.notifyListener(com.google.android.gms:play-services-location@@20.0.0:2)
at com.google.android.gms.common.api.internal.ListenerHolder.zaa(com.google.android.gms:play-services-base@@18.0.1:2)
at com.google.android.gms.common.api.internal.zacb.run(:4)
at android.os.Handler.handleCallback(Handler.java:938)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:236)
at android.app.ActivityThread.main(ActivityThread.java:8057)
at java.lang.reflect.Method.invoke(Method.java)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:656)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:967)

It is easily reproducible with example app in react-native-geolcation repo.
Reproducer App: https://github.com/michalchudziak/react-native-geolocation/tree/master/example

Steps to repro:

  • Clone the example app.
  • Make Two Calls to getCurrentPosition
  Geolocation.getCurrentPosition(
    (info) => {
      console.log('GetCurrentPosition Success', JSON.stringify(info));
    },
    (error) => {
      console.log('GetCurrentPosition Error', JSON.stringify(error));
    },
    {
      enableHighAccuracy: true,
      maximumAge: 20 * 1000,
      timeout: 10 * 1000,
    }
  );

  Geolocation.getCurrentPosition(
    (info) => {
      console.log('GetCurrentPosition Success', JSON.stringify(info));
    },
    (error) => {
      console.log('GetCurrentPosition Error', JSON.stringify(error));
    },
    {
      enableHighAccuracy: true,
      maximumAge: 20 * 1000,
      timeout: 10 * 1000,
    }
  );
  • Run the Example App:
    • Select the playServices option.
    • Click the getCurrentPosition button.
  • The app crashes with the stack trace provided above.

Issue Summary

The mSingleLocationCallback gets overwritten when multiple calls to getCurrentPosition() are made before the previous call finishes. This leads to a situation where null is passed to mFusedLocationClient.removeLocationUpdates(), causing the Listener must not be null exception.

Changes Made:

  • Introduced a List (mPendingLocationCallbacks) to keep track of pending location callbacks. This ensures that callbacks are properly managed and removed, preventing the crash.
  • Updated getCurrentLocationData, startObserving, and stopObserving methods to handle the addition and removal of callbacks more safely and consistently.

Test Plan

Tested getCurrentPosition and watchPosition functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@shubhamguptadream11