customerio / customerio-android

This is the official Customer.io SDK for Android.
MIT License
11 stars 9 forks source link

DiskReadViolation #360

Open laurentlr opened 2 weeks ago

laurentlr commented 2 weeks ago

SDK version: 3.10.0 Environment: Development & Production

Are logs available?

StrictMode policy violation; ~duration=10 ms: android.os.strictmode.DiskReadViolation
        at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1661)
        at libcore.io.BlockGuardOs.fstat(BlockGuardOs.java:194)
        at libcore.io.ForwardingOs.fstat(ForwardingOs.java:261)
        at libcore.io.IoBridge.open(IoBridge.java:563)
        at java.io.FileOutputStream.<init>(FileOutputStream.java:236)
        at java.io.FileOutputStream.<init>(FileOutputStream.java:186)
        at kotlin.io.e.d(FileReadWrite.kt:20)
        at io.customer.sdk.data.store.h.c(FileStorage.kt:41)
        at io.customer.sdk.queue.QueueStorageImpl.a(QueueStorage.kt:95)
        at io.customer.sdk.queue.QueueImpl.i(Queue.kt:68)
        at io.customer.sdk.queue.QueueImpl.c(Queue.kt:39)
        at pf.h.b(TrackRepository.kt:66)
        at ef.c.c(PushMessageProcessorImpl.kt:50)
        at io.customer.messagingpush.CustomerIOCloudMessagingReceiver.onReceive(CustomerIOCloudMessagingReceiver.kt:37)
        at android.app.ActivityThread.handleReceiver(ActivityThread.java:4663)
        at android.app.ActivityThread.-$$Nest$mhandleReceiver(Unknown Source:0)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2357)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loopOnce(Looper.java:232)
        at android.os.Looper.loop(Looper.java:317)
        at android.app.ActivityThread.main(ActivityThread.java:8501)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:878)

Describe the bug With StrictMode enabled, when sending a push notification, we can see DiskReadViolation. It's coming from your class CustomerIOCloudMessagingReceiver

To Reproduce

Expected behavior There should not be any DiskReadViolation on main thread.

mrehan27 commented 2 weeks ago

Hey @laurentlr. Thank you for reaching out and sharing your feedback. Because push notifications can be received when the app is in killed state and the OS can limit resources and code execution while the app is still in the background, to prevent metrics from being ignored, we process them to store and ensure they are not lost, hence resulting in the violation.

I agree this isn't ideal behavior and we have noted this down and do have plans to improve it. While I can't share specific time frame for the improvement to be released at the moment, I can confirm that it's something we have under consideration.

Thanks for sharing. Please don't hesitate to ask any further questions or share more feedback. Your input is important for us to improve our product. Thank you for sharing, and have a great day.

laurentlr commented 1 week ago

Hi @mrehan27, I'm reproducing this issue when app is in foreground (haven't tried with app killed).

we process them to store and ensure they are not lost

That's fine, but this storage processing should be done on the IO thread (like any storage in general) and not on the main thread.

mrehan27 commented 1 week ago

Thanks for the confirmation @laurentlr. It indeed will be reproducible when the app is in foreground because the code executed when push is received is same in both app opened and killed states.

I agree this should not be done on main thread. However, the change may not be as trivial as it looks since this will make the code run asynchronously and not return results instantly. To avoid any unwanted bugs, we need to ensure we improve this without affecting any of the current behavior.

Having said that, I do want to reconfirm that this is noted and something we plan to include in our roadmap this year. However, we are not working on this actively so I can't confirm any expected date for the changes at this moment.

We'll keep you posted about any progress on this. Thanks again for highlighting and sharing the details.