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

StrictMode Violation #652

Closed
commonsguy opened this issue Mar 11, 2018 · 7 comments · Fixed by #653
Closed

StrictMode Violation #652

commonsguy opened this issue Mar 11, 2018 · 7 comments · Fixed by #653
Assignees

Comments

@commonsguy
Copy link

commonsguy commented Mar 11, 2018

I was working on a sample showing how we can use the new penaltyListener() feature of StrictMode in Android P for logging StrictMode violations via ACRA. The good news is, it works! The bad news is, I found out not by my network violation... but ACRA's own disk I/O on the main application thread as part of startup:

E/AndroidRuntime: FATAL EXCEPTION: main
  Process: com.commonsware.android.acra.strictmode, PID: 6780
  java.lang.RuntimeException: StrictMode ThreadPolicy violation
     at android.os.StrictMode$AndroidBlockGuardPolicy.onThreadPolicyViolation(StrictMode.java:1654)
     at android.os.StrictMode$AndroidBlockGuardPolicy.handleViolationWithTimingAttempt(StrictMode.java:1511)
     at android.os.StrictMode$AndroidBlockGuardPolicy.startHandlingViolationException(StrictMode.java:1482)
     at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1453)
     at java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:251)
     at java.io.File.exists(File.java:815)
     at android.app.ContextImpl.getDataDir(ContextImpl.java:2207)
     at android.app.ContextImpl.getDir(ContextImpl.java:2226)
     at android.content.ContextWrapper.getDir(ContextWrapper.java:293)
     at org.acra.file.ReportLocator.getUnapprovedFolder(ReportLocator.java:45)
     at org.acra.file.ReportLocator.getUnapprovedReports(ReportLocator.java:50)
     at org.acra.file.BulkReportDeleter.deleteReports(BulkReportDeleter.java:46)
     at org.acra.util.ApplicationStartupProcessor.lambda$checkReports$0$ApplicationStartupProcessor(ApplicationStartupProcessor.java:58)
     at org.acra.util.ApplicationStartupProcessor$$Lambda$0.run(Unknown Source:4)
     at android.os.Handler.handleCallback(Handler.java:819)
     at android.os.Handler.dispatchMessage(Handler.java:99)
     at android.os.Looper.loop(Looper.java:164)
     at android.app.ActivityThread.main(ActivityThread.java:6656)
     at java.lang.reflect.Method.invoke(Native Method)
     at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:823)
  Caused by: android.os.strictmode.DiskReadViolation
     at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1453) 
     at java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:251) 
     at java.io.File.exists(File.java:815) 
     at android.app.ContextImpl.getDataDir(ContextImpl.java:2207) 
     at android.app.ContextImpl.getDir(ContextImpl.java:2226) 
     at android.content.ContextWrapper.getDir(ContextWrapper.java:293) 
     at org.acra.file.ReportLocator.getUnapprovedFolder(ReportLocator.java:45) 
     at org.acra.file.ReportLocator.getUnapprovedReports(ReportLocator.java:50) 
     at org.acra.file.BulkReportDeleter.deleteReports(BulkReportDeleter.java:46) 
     at org.acra.util.ApplicationStartupProcessor.lambda$checkReports$0$ApplicationStartupProcessor(ApplicationStartupProcessor.java:58) 
     at org.acra.util.ApplicationStartupProcessor$$Lambda$0.run(Unknown Source:4) 
     at android.os.Handler.handleCallback(Handler.java:819) 
     at android.os.Handler.dispatchMessage(Handler.java:99) 
     at android.os.Looper.loop(Looper.java:164) 
     at android.app.ActivityThread.main(ActivityThread.java:6656) 
     at java.lang.reflect.Method.invoke(Native Method) 
     at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438) 
     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:823) 

I'm just calling ACRA.init(this); in attachBaseContext(), per the docs, using ACRA 5.1.1. I would assume it's ACRA's job to do that work on a background thread, particularly since the stack trace doesn't show signs of my code.

Attached is the project that generated the above stack trace. I switched StrictMode to penaltyDeath() instead of penaltyListener(), though you could re-enable that if you wanted. As it stands, the app crashes on startup with the above StrictMode violation.
StrictMode-20180311.zip

Let me know if you need more info!

@F43nd1r
Copy link
Member

F43nd1r commented Mar 15, 2018

I've moxed the init code to a new thread. However, ACRA will still make disk I/O on the main thread, if the main thread just crashed.

I don't see a point to move this out to a secondary thread, as responsiveness cannot be achieved because of the crash.

@commonsguy
Copy link
Author

One reason to move it to a secondary thread is so that you do not have a StrictMode violation while trying to process the existing crash. Otherwise, while processing one crash, you may crash again due to the I/O.

@F43nd1r
Copy link
Member

F43nd1r commented Mar 15, 2018

While I see where you're coming from, I don't think compatibility with a debug tool justifies implementing an otherwise useless performance overhead.
As mentioned in the StrictMode docs "But don't feel compelled to fix everything that StrictMode finds."

@commonsguy
Copy link
Author

You might want to document that ACRA may have compatibility issues with StrictMode with penaltyDeath() on whatever actions that ACRA will take on the main application thread (disk reads? disk writes?).

In principle, this shouldn't be a problem in production, as you shouldn't be using penaltyDeath(). However, somebody might have a build type where they have StrictMode enabled with penaltyDeath() and also have ACRA configured and initialized. Then, they may be in a world of hurt if they get a crash.

Unfortunately, StrictMode has a write-only API, so we can't read the settings and take evasive action.

@F43nd1r
Copy link
Member

F43nd1r commented Mar 15, 2018

disk reads? disk writes?

Writes. More specifically, we're writing out the crash report https://github.com/ACRA/acra/blob/master/acra-core/src/main/java/org/acra/builder/ReportExecutor.java#L262

I've just seen allowThreadDiskWrites() in the docs. I think calling this prior to the disk write should be a negligible overhead.

@commonsguy
Copy link
Author

Ah, I forgot that allowThreadDiskWrites() returned the existing policy, so you can reinstate it afterwards. That seems like it'll work. Thanks!

@F43nd1r
Copy link
Member

F43nd1r commented Mar 24, 2018

Reopen because I've noticed several cases where disk read/writes are done on the main thread.
I hope I caught everything now.

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

Successfully merging a pull request may close this issue.

2 participants