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

Null pointer crash processing rest client response #1514

Closed
joerogers opened this issue Jan 13, 2022 · 8 comments
Closed

Null pointer crash processing rest client response #1514

joerogers opened this issue Jan 13, 2022 · 8 comments

Comments

@joerogers
Copy link

joerogers commented Jan 13, 2022

Description:

OneSignal is crashing the app with this stack. Has happened 9 times in last week since we deployed our app with OneSignal the first time.

Fatal Exception: java.lang.NullPointerException
       at java.util.Objects.requireNonNull(Objects.java:203)
       at com.onesignal.OneSignal.init(OneSignal.java:837)
       at com.onesignal.OneSignal.setAppId(OneSignal.java:692)
       at com.onesignal.OneSignal.reassignDelayedInitParams(OneSignal.java:1136)
       at com.onesignal.OneSignal.onRemoteParamSet(OneSignal.java:844)
       at com.onesignal.OneSignal$6.complete(OneSignal.java:1077)
       at com.onesignal.OneSignalRemoteParams.processJson(OneSignalRemoteParams.java:206)
       at com.onesignal.OneSignalRemoteParams.access$100(OneSignalRemoteParams.java:12)
       at com.onesignal.OneSignalRemoteParams$1.onSuccess(OneSignalRemoteParams.java:151)
       at com.onesignal.OneSignalRestClient$5.run(OneSignalRestClient.java:283)
       at java.lang.Thread.run(Thread.java:919)

I believe what is happening taking a cursory look at the logic.

  1. OneSignal is initializing.
  2. It requests the remote params are initialized
  3. The initial code running the initializer is suspended by the OS (these things happen)
  4. The http request completes which also calls init.
  5. However, the context is already set so it assumes the full initialization is complete
  6. Crashes trying to access the following line because outcomeEventsController is still null
outcomeEventsController.sendSavedOutcomes();

Wrapping this with a null check should still work as if my theory is right, the original init call will complete and invoke it, but now outcomeEventsController will not be null.

Environment

  1. What version of the Android SDK are you using?
    4.6.3 (but doesn't look like the code changed in 4.6.5)

  2. How did you add the SDK to your project (eg. maven)
    Gradle via maven

Steps to Reproduce Issue:

Unknown, nothing else in the stack and very little in production logs which implies this is during startup.

Anything else:

See above

@emawby
Copy link
Contributor

emawby commented Jan 13, 2022

@joerogers Thank you for reporting. Have you had any luck reproducing this crash? If not we can at least try wrapping that controller in a null check like you mention

@joerogers
Copy link
Author

joerogers commented Jan 14, 2022

@emawby This is a production only crash. I have never seen it in development. If you wrote in Kotlin, this logic would have been flagged as a potential null pointer based on declaration + usage. Essentially there is no guarantee outcomeEventsController is not null based on the initialization could be bypassed or delayed in this logic in initWithContext()

      boolean wasAppContextNull = (appContext == null);
      appContext = context.getApplicationContext();

      setupContextListeners(wasAppContextNull);

My theory is setupContextListeners() is delayed when it reads/writes shared preferences for language if wasAppContextNull == true which are io operations and may block the calling thread until complete. This happens before the outcomeEventsController is assigned. In the meantime the network call came back and starts a race condition by calling init() since appContext != null. Init() assumes initWithContext() was invoked and setupContextListeners() had completed.

An alternate solution is to not set the appContext until after any key setup has completed, OR use a boolean flag that init checks before invoking.

I do see you check outcomeEventsController for null in several other places so I have to think that was done because you have had nulls before.

@azharfauzandz
Copy link

azharfauzandz commented Feb 17, 2022

It's happened too in our apps with the same stack. We are using One Signal with version 4.6.0 for the first time and we updated it to 4.6.7 but still happened. It was only about < 2% of users that were affected. Is there any update on this issue? Thank you

@nan-li
Copy link
Contributor

nan-li commented Feb 23, 2022

This looks like the same issue as #1374.

@nan-li
Copy link
Contributor

nan-li commented Feb 25, 2022

Hi @joerogers, thank you for sharing your investigation. We are working on a fix, and can you and @azharfauzandz share what type of devices and Android versions are seeing this crash? And is the app crashing in the background or foreground?

@joerogers
Copy link
Author

@nan-li I have see this both foreground 70% and background 30% but always at app startup, since this is an initialization call.

I'm seeing on all sort of phones and Android versions.

Top phone makers are Samsung and Huawei (but all the major players are there)
Top OS versions are 10, 11, and 9. (but our app runs 6+ and has occurred on all versions)

@azharfauzandz
Copy link

Hi @nan-li thank you for the responses

Crash happened in Background 99% and Foreground 1% (Mostly happened in the first 1-3 seconds of the user’s session.).
Top device: OPPO, VIVO, Realme, Xiaomi, Samsung
Top OS versions: 10, 11, 8.

@nan-li
Copy link
Contributor

nan-li commented Apr 8, 2022

Thank you for waiting @joerogers @azharfauzandz,

A fix has been released in version 4.7.1.

I will close this issue for now, please upgrade and reach out if you are still experiencing crashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants