Strict mode violation in crash dump uploader. |
||||
Issue descriptionChrome Dev 59.0.3068.4 triggers strict mode violation when trying to upload a crash dump. Device: Android 7.1.2; Nexus 6P Build/N2G05 04-21 10:27:42.864 19547 19680 I cr_MinidumpJobService: Scheduling upload of all pending minidumps. 04-21 10:27:42.923 19547 19806 I cr_CrashFileManager: /data/user/0/com.chrome.dev/cache/Crash Reports 04-21 10:27:42.928 19547 19547 D StrictMode: StrictMode policy violation; ~duration=12 ms: android.os.StrictMode$StrictModeDiskReadViolation: policy=18153503 violation=2 04-21 10:27:42.928 19547 19547 D StrictMode: at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1293) 04-21 10:27:42.928 19547 19547 D StrictMode: at java.io.UnixFileSystem.checkAccess(UnixFileSystem.java:249) 04-21 10:27:42.928 19547 19547 D StrictMode: at java.io.File.exists(File.java:780) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.app.ContextImpl.getDataDir(ContextImpl.java:1938) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.app.ContextImpl.getCacheDir(ContextImpl.java:582) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.content.ContextWrapper.getCacheDir(ContextWrapper.java:253) 04-21 10:27:42.928 19547 19547 D StrictMode: at org.chromium.chrome.browser.crash.ChromeMinidumpUploaderDelegate.getCrashParentDir(ChromeMinidumpUploaderDelegate.java:7) 04-21 10:27:42.928 19547 19547 D StrictMode: at org.chromium.components.minidump_uploader.MinidumpUploaderImpl.<init>(MinidumpUploaderImpl.java:4) 04-21 10:27:42.928 19547 19547 D StrictMode: at org.chromium.chrome.browser.crash.ChromeMinidumpUploadJobService.createMinidumpUploader(ChromeMinidumpUploadJobService.java:2) 04-21 10:27:42.928 19547 19547 D StrictMode: at org.chromium.components.minidump_uploader.MinidumpUploadJobService.onStartJob(MinidumpUploadJobService.java:15) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.app.job.JobService$JobHandler.handleMessage(JobService.java:143) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.os.Handler.dispatchMessage(Handler.java:102) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.os.Looper.loop(Looper.java:154) 04-21 10:27:42.928 19547 19547 D StrictMode: at android.app.ActivityThread.main(ActivityThread.java:6119) 04-21 10:27:42.928 19547 19547 D StrictMode: at java.lang.reflect.Method.invoke(Native Method) 04-21 10:27:42.928 19547 19547 D StrictMode: at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886) 04-21 10:27:42.928 19547 19547 D StrictMode: at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776) 04-21 10:27:42.929 19547 19547 D AndroidRuntime: Shutting down VM 04-21 10:27:42.955 19547 19806 I cr_MinidumpUploaderImpl: Attempting to upload 1 minidumps. 04-21 10:27:42.955 19547 19806 I cr_MinidumpUploaderImpl: Attempting to upload 103eaf37-6e55-d97b-0726baae-760f8629.dmp19257.try0 04-21 10:27:43.210 19547 19547 W google-breakpad: Output crash dump file: 04-21 10:27:43.210 19547 19547 W google-breakpad: /data/user/0/com.chrome.dev/cache/Crash Reports/22604af8-ef62-7370-4dda3d26-02da2522.dmp 04-21 10:27:43.210 19547 19547 W google-breakpad: ### ### ### ### ### ### ### ### ### ### ### ### ### 04-21 10:27:43.210 19547 19547 W google-breakpad: Chrome build fingerprint: 04-21 10:27:43.210 19547 19547 W google-breakpad: 59.0.3068.4 04-21 10:27:43.210 19547 19547 W google-breakpad: 306800452 04-21 10:27:43.210 19547 19547 W google-breakpad: ### ### ### ### ### ### ### ### ### ### ### ### ### 04-21 10:27:43.220 19547 19684 I cr_LogcatExtraction: Trying to extract logcat for minidump 22604af8-ef62-7370-4dda3d26-02da2522.dmp19547. 04-21 10:27:43.267 19810 19547 F google-breakpad: -----BEGIN BREAKPAD MICRODUMP----- 04-21 10:27:43.267 19810 19547 F google-breakpad: V Chrome_Android:59.0.3068.4 04-21 10:27:43.267 19810 19547 F google-breakpad: O A arm 08 armv8l google/angler/angler:7.1.2/N2G05/3614058:userdebug/dev-keys 04-21 10:27:43.267 19810 19547 F google-breakpad: P browser 04-21 10:27:43.267 19810 19547 F google-breakpad: G UNKNOWN 04-21 10:27:43.268 19810 19547 F google-breakpad: H 12C00000 FFFF1000 0048 3226C000 A2EF6000 0C:12 0D:17 0E:0D 0F:03 11:01 12:04 13:02 14:01 16:01 1B:02 1C:02 1D:02 04-21 10:27:43.268 19810 19547 F google-breakpad: S 0 FF911B28 FF911000 0000400
,
Apr 21 2017
Yep, AsyncTask should help.
,
Apr 21 2017
Ah, I'd always wondered why we had file operations on the UI thread... I'd kind of assumed it was intentional. (This code predates both myself and Gustav having worked on it, for the most part.)
,
Apr 21 2017
Oh, I assumed that jobs (like services) ran on a non-UI thread. Surprised! We should definitely not be doing any crash uploading on UI thread.
,
Apr 21 2017
The upload itself does happen on a worker thread [1] (whew!). However, there are some file operations that we're performing on the UI thread. [1] https://cs.chromium.org/chromium/src/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploaderImpl.java?l=157-158
,
Apr 24 2017
The only file operations we perform on the UI thread here should be listing files - once to set up the CrashFileManager (when calling MinidumpUploaderDelegate.getCrashParentDir() from the MinidumpUploaderImpl ctor), and once to know whether there are any minidumps left at the end of an uploading-session (when calling mFileManager.getAllMinidumpFiles() from MinidumpUploaderImpl.cancelUploads()).
We could probably just move the creation of the CrashFileManager onto the worker-thread. Then the only place where we perform file-operations on the UI thread should be in MinidumpUploaderImpl.cancelUploads(). We should make sure that our job is rescheduled if it needs to be - and we need to know whether to reschedule within the MinidumpUploaderImpl.cancelUploads() callback - which is performed on the UI thread.
Given that we now start a new minidump-uploading job unconditional on whether the last one finished, we should be able to synchronize a variable declaring whether the last job finished between the worker thread and the UI thread (we'll have to be careful with jobs that we don't care about anymore but which are still alive though - so that they don't update the new variable incorrectly).
Something like this maybe:
Worker thread:
uploadMinidumps():
synchronized isDone = (getNumberOfMinidumps() == 0)
for minidump in minidumps:
upload minidump
synchronized isDone = (getNumberOfMinidumps() == 0)
UI Thread:
cancelUploads():
synchronized if (!isDone) reschedule()
,
Apr 25 2017
Is there a way to turn on strictmode for Chrome with a flag or something? (rather than having to programmatically enable it sometime during startup).
,
Apr 25 2017
Ted and Dan: How to turn on strict mode for Gustav?
,
Apr 25 2017
Thought it was built in automatically. Other than asking wnwen@, I've only turned on red strict mode flashing in the Developer Options from Android's settings.
,
Apr 25 2017
Thanks Ted!
,
May 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee63feacd7199214bf9673efd957afabe9be13bb commit ee63feacd7199214bf9673efd957afabe9be13bb Author: gsennton <gsennton@chromium.org> Date: Wed May 03 13:15:59 2017 Move Minidump Uploading IO operations off the UI thread. Before this CL we would create a CrashFileManager in MinidumpUploaderImpl on the UI thread. CrashFileManager requires a directory in which to store minidumps, so creating that directory, or ensuring that it exists, requires disk operations. In this CL we move the creation of the CrashFileManager onto the worker thread where the actual minidump uploading happens. Also change the rescheduling logic for minidump uploading slightly, Previously we would check whether there are any minidumps left, at the time when the current job is cancelled, from the UI thread. But that requires a disk operation, so now we instead update whether to reschedule uploads, from the worker thread, for the UI thread to read in a thread-safe way (without disk reads). Note that this change affects the minidump uploading of both WebView and Chrome. BUG= 714138 Review-Url: https://codereview.chromium.org/2838383002 Cr-Commit-Position: refs/heads/master@{#468955} [modify] https://crrev.com/ee63feacd7199214bf9673efd957afabe9be13bb/android_webview/java/src/org/chromium/android_webview/crash/AwMinidumpUploaderDelegate.java [modify] https://crrev.com/ee63feacd7199214bf9673efd957afabe9be13bb/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java [modify] https://crrev.com/ee63feacd7199214bf9673efd957afabe9be13bb/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/CrashFileManager.java [modify] https://crrev.com/ee63feacd7199214bf9673efd957afabe9be13bb/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploaderImpl.java
,
May 3 2017
,
May 5 2017
Issue 718879 has been merged into this issue. |
||||
►
Sign in to add a comment |
||||
Comment 1 by gsennton@chromium.org
, Apr 21 2017