project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.33k stars 1.97k forks source link

[BUG] java.lang.NullPointerException: Attempt to invoke interface method 'boolean java.util.concurrent.ScheduledFuture.cancel(boolean)' on a null object reference #26644

Open jonsmirl opened 1 year ago

jonsmirl commented 1 year ago

Reproduction steps

Unknown what triggers this. I hit it a couple times a week.

2023-05-17 18:36:28.971  9950-10039 AndroidRuntime          com.lowpan.m2                        E  FATAL EXCEPTION: NsdManager
                                                                                                    Process: com.lowpan.m2, PID: 9950
                                                                                                    java.lang.NullPointerException: Attempt to invoke interface method 'boolean java.util.concurrent.ScheduledFuture.cancel(boolean)' on a null object reference
                                                                                                        at chip.platform.NsdServiceFinderAndResolver.onServiceFound(NsdServiceFinderAndResolver.java:102)
                                                                                                        at android.net.nsd.NsdManager$ServiceHandler.lambda$handleMessage$2(NsdManager.java:694)
                                                                                                        at android.net.nsd.NsdManager$ServiceHandler$$ExternalSyntheticLambda4.run(Unknown Source:4)
                                                                                                        at android.net.nsd.NsdManager$$ExternalSyntheticLambda0.execute(Unknown Source:0)
                                                                                                        at android.net.nsd.NsdManager$ServiceHandler.handleMessage(NsdManager.java:694)
                                                                                                        at android.os.Handler.dispatchMessage(Handler.java:106)
                                                                                                        at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                        at android.os.Looper.loop(Looper.java:288)
                                                                                                        at android.os.HandlerThread.run(HandlerThread.java:67)

This line "if (stopDiscoveryRunnable.cancel(false)) {" is not checking to ensure stopDiscoveryRunnable is not null first before using it.

  @Override
  public void onServiceFound(NsdServiceInfo service) {
    if (targetServiceInfo.getServiceName().equals(service.getServiceName())) {
      Log.d(TAG, "onServiceFound: found target service " + service);

      if (stopDiscoveryRunnable.cancel(false)) {
        nsdManager.stopServiceDiscovery(this);
        if (multicastLock.isHeld()) {
          multicastLock.release();
        }
      }

      if (nsdManagerResolverAvailState != null) {
        nsdManagerResolverAvailState.acquireResolver();
      }

      resolveService(service, callbackHandle, contextHandle, chipMdnsCallback, timeoutRunnable);
    } else {
      Log.d(TAG, "onServiceFound: found service not a target for resolution, ignoring " + service);
    }
  }

Bug prevalence

2-3 times a week

GitHub hash of the SDK that was being used

bd17b9f52f41eeac5aaa74c1ecf3a76e06dd6958

Platform

android

Platform Version(s)

No response

Anything else?

No response

joonhaengHeo commented 1 year ago

I can't tell you exactly as I haven't checked this issue, but it seems the 2 code locations below need to be changed.

Just looking at the log, it is judged that onServiceFound is called immediately after discoverServices, so it is thought that stopDiscoveryRunnable should be set first before discoverServices called.

https://github.com/project-chip/connectedhomeip/blob/master/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java#L74

https://github.com/project-chip/connectedhomeip/blob/master/src/platform/android/java/chip/platform/NsdServiceFinderAndResolver.java#L78

@yunhanw-google @yufengwangca What is your opinion?

jonsmirl commented 1 year ago

This bug appears to be some kind of small timing window. I can run this code hundreds of times and never hit this. Then I will hit it once. Then it will work again for another hundred times.

jonsmirl commented 1 year ago

Not sure, but this seems to be correlated to bringing app out of the background up to foreground on Android. I was working on my app, I got a notice about important email, I looked at the email. Then I went back to CHIP app -- bang I hit it.

Have MDNS messages piled up while the CHIP app was in background, then when I brought it to foreground they were processed in this vulnerable window?

yunhanw-google commented 6 months ago

@sharadb-amazon any idea for this one?