microsoft / appcenter-sdk-android

Development repository for the App Center SDK for Android
Other
276 stars 134 forks source link

Fix NPE in Distribute module on resume workflow #1663

Closed shadyabarada closed 1 year ago

shadyabarada commented 1 year ago

Please have a look at our guidelines for contributions and consider the following before you submit the PR:

Description

My initial thoughts around this issue: The issue is happening under the scenario of which the distribute SDK needs to show the "Update Setup Failed" dialog. When the "showUpdateSetupFailedDialog" method is called, the alert dialog uses the global variable activity instance mForegroundActivity. The mForegroundActivity in the above scenario is null. Thus the crash. How could this variable be null? Let's observe the workflow: In our Distribute.java class, mForegroundActivity is a global private variable used in the scope of this class. The value of the mForegroundActivity is set under the following two cases: onActivityResumed => The value of mForegroundActivity is set to the value of the activity being resumed. onActivityPaused => The value of mForegroundActivity is explicitly set to null. The call for "showUpdateSetupFailedDialog" occurs inside the "resumeDistributeWorkflow" function which is called by the onActivityResumed method. The "resumeDistributeWorkflow" has a condition on its very first line which checks if our mForegroundActivity is null, and apparently, this check passes before reaching the line of code that should show the "update failed" alert dialog where our code is crashing. Therefore, the value of mForegroundActivity has been tampered with along the way before reaching the "show update failed" dialog. Weird right? We are within the same block of code, under the same method, and between few lines of code that do not explicitly change the value of our mForegroundActivity, the value has actually changed. This issue is most probably a "thread shared memory issue". I am assuming that one of the running threads have executed the onActivityPause function which updates the value of the mForegroundActivity to null, while on onActivityResume code is being executed. Thus failing to show the alert dialog and producing the above null pointer exception. I will be investigating further and trying to create a small test with multithreads trying to access and update global variables, and look into possible solutions.

Note that the user which this issue is occurring with, might be stuck in the loop of crashes every time he/she opens their app because of having the values with the running threads continually stored in memory - I will reply on the GitHub conversation thread for this issue asking if they can try to clear app cache and data for their app from the device's settings and try again to see if the issue continuous.

Related PRs or issues

https://dev.azure.com/msmobilecenter/Mobile-Center/_workitems/edit/95531

Misc

Add what's missing, notes on what you tested, additional thoughts or questions.