Fix ANR when initializing FontLoader#2819
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2819 +/- ##
==========================================
- Coverage 78.24% 78.24% -0.01%
==========================================
Files 325 325
Lines 12728 12730 +2
Branches 1738 1739 +1
==========================================
+ Hits 9959 9960 +1
Misses 2038 2038
- Partials 731 732 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @OptIn(InternalRevenueCatAPI::class) | ||
| internal class FontLoader( | ||
| private val context: Context, | ||
| private val cacheDir: File = File(context.cacheDir, "rc_paywall_fonts"), |
There was a problem hiding this comment.
Hmm does creating a File pointer actually perform disk operations? I believe this just creates a pointer, but doesn't actually interact with disk, until you actually want to perform some operations (which I think should be happening in the ioScope?)
There was a problem hiding this comment.
Right... I think you're correct 🤔
There was a problem hiding this comment.
What if the issue is context.cacheDir?
Looking at the stacktrace it looks like it does some checks?
at android.app.ActivityThread$AndroidOs.access(ActivityThread.java:8582)
at java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:332)
at java.io.File.exists(File.java:829)
at android.app.ContextImpl.ensurePrivateDirExists(ContextImpl.java:819)
at android.app.ContextImpl.ensurePrivateCacheDirExists(ContextImpl.java:815)
at android.app.ContextImpl.getCacheDir(ContextImpl.java:926)
at android.content.ContextWrapper.getCacheDir(ContextWrapper.java:328)
There was a problem hiding this comment.
For reference https://android.googlesource.com/platform/frameworks/base/+/HEAD/core/java/android/app/ContextImpl.java#822
I did check and this PR's changes removes the StrictMode warnings
There was a problem hiding this comment.
ohhh right... my guess would be that indeed it has something to do with context.cacheDir as you said. I guess that's checking for the cache dir existance, and so performing disk operations... Good to know!
There was a problem hiding this comment.
btw there are so many other warnings with StrictMode related to shared preferences, but that's another story :D
There was a problem hiding this comment.
checking for the cache dir existance
Yes, I've seen that.
RE StrictMode, I think we should enable it in our Maestro test app (and fix all errors). But indeed, different story.
JayShortway
left a comment
There was a problem hiding this comment.
Nice, thanks for fixing this!
tonidero
left a comment
There was a problem hiding this comment.
Looks good! Just a question
| ) { | ||
| private var hasCheckedFoldersExist: AtomicBoolean = AtomicBoolean(false) | ||
|
|
||
| private val cacheDirectory by lazy(LazyThreadSafetyMode.NONE) { |
There was a problem hiding this comment.
Should we use a synchronized safety mode? It shouldn't happen that we initialize this from multiple threads... But might be slightly safer.
There was a problem hiding this comment.
I actually don't think that would add any safety since there is no need to lock anything. If two threads hit this at the same time without locking they would just return a File pointing to the same directory, since File(context.cacheDir, "rc_paywall_fonts") is idempotent. And if they don't create the File, they would just return providedCacheDir which is also not necessary to synchronize since it's just a get
**This is an automatic release.** > [!WARNING] > If you don't have any login system in your app, please make sure your one-time purchase products have been correctly configured in the RevenueCat dashboard as either consumable or non-consumable. If they're incorrectly configured as consumables, RevenueCat will consume these purchases. This means that users won't be able to restore them from version 9.0.0 onward. > Non-consumables are products that are meant to be bought only once, for example, lifetime subscriptions. ## RevenueCatUI SDK ### 🐞 Bugfixes * Fix ANR when initializing FontLoader (#2819) via Cesar de la Vega (@vegaro) ### Paywallv2 #### 🐞 Bugfixes * Fix `Template7CustomPackagesTestData` (#2875) via Cesar de la Vega (@vegaro) * Fix predownloading of fonts if first offering doesn't have paywall components (#2873) via Cesar de la Vega (@vegaro) ### 🔄 Other Changes * Extract parameters for non paid revenue tracking API to use objects (#2871) via Toni Rico (@tonidero) * Bump fastlane from 2.229.0 to 2.229.1 (#2869) via dependabot[bot] (@dependabot[bot]) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Reported here #2818
I believe the cause is we are doing
context.cacheDiron initialization (in main thread). Looking at the stacktrace it looks like it does some checks on the disk on main thread that could be causing ANRs.I tested with StrictMode and after this fix, the warning is gone.