search

Automattic / pocket-casts-android

Pocket Casts Android 🎧
https://forums.pocketcasts.com
Mozilla Public License 2.0
2.59k stars 225 forks source link

Crash on logout race condition #958

Open mchowning opened 1 year ago

mchowning commented 1 year ago

Description

In the watch app I have observed that occasionally logging out will cause a crash because it tries to get an access token here, but fails because the refresh token is null. This appears to be a race condition since there is an earlier guard on this code confirming that the user is logged in.

I have only reproduced this on the watch app, but this is using the same logic as the phone app, so the same problem might be possible there. I have not been able to reproduce this on the phone app though.

Step-by-step reproduction instructions

I cannot reproduce this consistently, but the steps I've been following taht seems to cause it the most consistently are (note that all of the details in these steps probably aren't necessary, I'm just reporting what I've been doing):

  1. Log into the watch app using google sign in
  2. Log out of the watch app
  3. If that doesn't crash, then log back into the google account and sign out again. On the watch app I get a crash within the first 2-3 signouts pretty consistently.

Screenshots or screen recording

No response

Did you search for existing bug reports?

Device, Operating system, and Pocket Casts app version

Watch emulator with a 7.39 pre-release build.

mchowning commented 1 year ago

I have confirmed that this is a race condition and that it can also occur on the phone app. It can be recreated consistently by applying a delay inside a Single.fromCallable call like this.

The problem seems to be that while the server call is being made inside a Single.fromCallable here the downstream disposes of the Single source. When that happens any Throwable that is thrown bypasses any error handling in the stream and instead goes straight to RxJava's global error handler which, on Android, crashes the app.

mchowning commented 1 year ago

https://github.com/Automattic/pocket-casts-android/pull/971 fixes the crash on release builds, but the underlying issue still exists, and it will still crash on debug builds. For that reason, I'm thinking it makes sense to leave this open. I don't feel strongly though, so let me know if you disagree.