pendo-io / pendo-mobile-sdk

Pendo captures product usage data, gathers user feedback, and lets you communicate in-app to onboard, educate, and guide users to value
https://www.pendo.io
Other
57 stars 2 forks source link

Pendo Android SDK violates StrictMode Policy #70

Closed rishabh876 closed 1 year ago

rishabh876 commented 1 year ago

Platform + Version Android 13

SDK Version 2.22.+ Framework Native

Describe the bug On enabling Strict Mode on android. It is noticed that Pendo is violating disk read/write policy by using sharedPreference.commit which does all IO operation on main thread synchronously.

To Reproduce Steps to reproduce the behavior:

  1. Enable strict mode in application onCreate
    public void onCreate() {
     StrictMode.setThreadPolicy(new [StrictMode.ThreadPolicy.Builder](https://developer.android.com/reference/android/os/StrictMode.ThreadPolicy.Builder)()
             .detectDiskReads()
             .detectDiskWrites()
             .detectNetwork()   // or .detectAll() for all detectable problems
             .penaltyLog()
             .build());
     StrictMode.setVmPolicy(new [StrictMode.VmPolicy.Builder](https://developer.android.com/reference/android/os/StrictMode.VmPolicy.Builder)()
             .detectLeakedSqlLiteObjects()
             .detectLeakedClosableObjects()
             .penaltyLog()
             .build());
     super.onCreate();
    ....
    }
  2. Call Pendo.startSession()
  3. Check android logcat for StrictMode violations.

Expected behavior MainThread should not be used for IO operation. While using shared preference, instead of commit(), apply() should be used.

Logs

StrictMode policy violation; ~duration=17 ms: android.os.strictmode.DiskWriteViolation at android.os.StrictMode$AndroidBlockGuardPolicy.onWriteToDisk(StrictMode.java:1614) at libcore.io.BlockGuardOs.chmod(BlockGuardOs.java:81) at libcore.io.ForwardingOs.chmod(ForwardingOs.java:165) at android.system.Os.chmod(Os.java:119) at android.os.FileUtils.setPermissions(FileUtils.java:168) at android.app.ContextImpl.setFilePermissionsFromMode(ContextImpl.java:3326) at android.app.SharedPreferencesImpl.writeToFile(SharedPreferencesImpl.java:812) at android.app.SharedPreferencesImpl.-$$Nest$mwriteToFile(Unknown Source:0) at android.app.SharedPreferencesImpl$2.run(SharedPreferencesImpl.java:672) at android.app.SharedPreferencesImpl.enqueueDiskWrite(SharedPreferencesImpl.java:691) at android.app.SharedPreferencesImpl.-$$Nest$menqueueDiskWrite(Unknown Source:0) at android.app.SharedPreferencesImpl$EditorImpl.commit(SharedPreferencesImpl.java:604) at sdk.pendo.io.i9.b0.a(SourceFile:0) at sdk.pendo.io.i9.z.d(SourceFile:0) at sdk.pendo.io.models.SessionData.persistData(SourceFile:0) at sdk.pendo.io.a.b(SourceFile:0) at sdk.pendo.io.a.a(SourceFile:0) at sdk.pendo.io.a.a(SourceFile:0) at sdk.pendo.io.a.$r8$lambda$SsLWDeZCQscnpsJUOCg-h25F5aI(SourceFile:0) at sdk.pendo.io.a$$ExternalSyntheticLambda8.accept(R8$$SyntheticClass:0) at sdk.pendo.io.f9.b.onSuccess(SourceFile:0) at sdk.pendo.io.z5.j$a.a(SourceFile:0) at sdk.pendo.io.e6.i.a(SourceFile:0) at sdk.pendo.io.k6.a$a.test(SourceFile:0) at sdk.pendo.io.k6.a$a.a(SourceFile:0) at sdk.pendo.io.k6.a.b(SourceFile:0) at sdk.pendo.io.l5.l.a(SourceFile:0) at sdk.pendo.io.z5.j.b(SourceFile:0) at sdk.pendo.io.l5.i.a(SourceFile:0) at sdk.pendo.io.a.a(SourceFile:0) at sdk.pendo.io.a.a(SourceFile:0) at sdk.pendo.io.Pendo.startSession(SourceFile:0)

Sample Code -NA

Additional context -NA

noambartouv commented 1 year ago

We appreciate you letting us know about this. We are looking into it and we will update on our findings.

noambartouv commented 1 year ago

@rishabh876 After careful investigation, it appears that the use of the commit() method is necessary for a particular edge case in which the apply() method does not produce the expected outcome. As this flow is not often called upon, it should not have a substantial impact on performance. The remainder of our code utilizes the apply() method.

We've considered refactoring the code and will take this into account for future updates. I've added it to the backlog but it is not likely to be part of our roadmap for the next few releases. For the time being, we will be closing this ticket.

We appreciate your input and sharing your insights. Thank you once again.