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

DiskReadViolation #360

Open
laurentlr opened this issue Jun 14, 2024 · 3 comments
Open

DiskReadViolation #360

laurentlr opened this issue Jun 14, 2024 · 3 comments

Comments

@laurentlr
Copy link

**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

StrictMode.setThreadPolicy(
                StrictMode.ThreadPolicy.Builder()
                    .detectDiskReads()
                    .detectDiskWrites()
                    .detectAll()
                    .penaltyLog()
                    .build()

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

@mrehan27
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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.

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

No branches or pull requests

2 participants