From fa178e14b991b9ff94c5360561893d3fde909794 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 11 Jun 2019 13:30:15 -0700 Subject: [PATCH 1/2] Frameworks: Change Java stack trace dumping When dumping stacks, e.g., for ANRs, even if the "Java" version from ART reports success, check that output was generated, and fall back to a "native" dump if the output is too small, currently set at one hundred bytes. Add more logging of failure paths. Fix some error logging in native to go to logcat instead of stderr. Bug: 133525168 Test: m Test: manual (induce ANRs with various platform changes) Exempt-From-Owner-Approval: Cherry-pick Merged-In: I0760207229989f6f34e10edcf45fb140bc81d0e5 Change-Id: I0760207229989f6f34e10edcf45fb140bc81d0e5 --- core/jni/android_os_Debug.cpp | 4 +-- .../server/am/ActivityManagerService.java | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/core/jni/android_os_Debug.cpp b/core/jni/android_os_Debug.cpp index 842679ec7d223..eb55a9309d283 100644 --- a/core/jni/android_os_Debug.cpp +++ b/core/jni/android_os_Debug.cpp @@ -713,7 +713,7 @@ static bool dumpTraces(JNIEnv* env, jint pid, jstring fileName, jint timeoutSecs O_CREAT | O_WRONLY | O_NOFOLLOW | O_CLOEXEC | O_APPEND, 0666)); if (fd < 0) { - fprintf(stderr, "Can't open %s: %s\n", fileNameChars.c_str(), strerror(errno)); + PLOG(ERROR) << "Can't open " << fileNameChars.c_str(); return false; } @@ -722,7 +722,7 @@ static bool dumpTraces(JNIEnv* env, jint pid, jstring fileName, jint timeoutSecs static jboolean android_os_Debug_dumpJavaBacktraceToFileTimeout(JNIEnv* env, jobject clazz, jint pid, jstring fileName, jint timeoutSecs) { - const bool ret = dumpTraces(env, pid, fileName, timeoutSecs, kDebuggerdJavaBacktrace); + const bool ret = dumpTraces(env, pid, fileName, timeoutSecs, kDebuggerdJavaBacktrace); return ret ? JNI_TRUE : JNI_FALSE; } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 0c7fe114fd409..f14a3fdda8dbe 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -547,6 +547,7 @@ public class ActivityManagerService extends IActivityManager.Stub private static final int MAX_BUGREPORT_TITLE_SIZE = 50; private static final int NATIVE_DUMP_TIMEOUT_MS = 2000; // 2 seconds; + private static final int JAVA_DUMP_MINIMUM_SIZE = 100; // 100 bytes. OomAdjuster mOomAdjuster; final LowMemDetector mLowMemDetector; @@ -3805,9 +3806,28 @@ public class ActivityManagerService extends IActivityManager.Stub */ private static long dumpJavaTracesTombstoned(int pid, String fileName, long timeoutMs) { final long timeStart = SystemClock.elapsedRealtime(); - if (!Debug.dumpJavaBacktraceToFileTimeout(pid, fileName, (int) (timeoutMs / 1000))) { - Debug.dumpNativeBacktraceToFileTimeout(pid, fileName, - (NATIVE_DUMP_TIMEOUT_MS / 1000)); + boolean javaSuccess = Debug.dumpJavaBacktraceToFileTimeout(pid, fileName, + (int) (timeoutMs / 1000)); + if (javaSuccess) { + // Check that something is in the file, actually. Try-catch should not be necessary, + // but better safe than sorry. + try { + long size = new File(fileName).length(); + if (size < JAVA_DUMP_MINIMUM_SIZE) { + Slog.w(TAG, "Successfully created Java ANR file is empty!"); + javaSuccess = false; + } + } catch (Exception e) { + Slog.w(TAG, "Unable to get ANR file size", e); + javaSuccess = false; + } + } + if (!javaSuccess) { + Slog.w(TAG, "Dumping Java threads failed, initiating native stack dump."); + if (!Debug.dumpNativeBacktraceToFileTimeout(pid, fileName, + (NATIVE_DUMP_TIMEOUT_MS / 1000))) { + Slog.w(TAG, "Native stack dump failed!"); + } } return SystemClock.elapsedRealtime() - timeStart; From 72768ed944a1348d77ad7ff300d6fc93f93e4827 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 11 Jun 2019 13:49:48 -0700 Subject: [PATCH 2/2] Frameworks: Add fdatasync after dumping a trace Ensure that data hits storage when dumping. Bug: 133525168 Test: m Test: manual (induce ANRs with various platform changes) Merged-In: I66eac0c5e77ea98ba1777d0803b35f38d9ca648f Change-Id: I66eac0c5e77ea98ba1777d0803b35f38d9ca648f --- core/jni/android_os_Debug.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/jni/android_os_Debug.cpp b/core/jni/android_os_Debug.cpp index eb55a9309d283..14dbabb1ce022 100644 --- a/core/jni/android_os_Debug.cpp +++ b/core/jni/android_os_Debug.cpp @@ -717,7 +717,11 @@ static bool dumpTraces(JNIEnv* env, jint pid, jstring fileName, jint timeoutSecs return false; } - return (dump_backtrace_to_file_timeout(pid, dumpType, timeoutSecs, fd) == 0); + int res = dump_backtrace_to_file_timeout(pid, dumpType, timeoutSecs, fd); + if (fdatasync(fd.get()) != 0) { + PLOG(ERROR) << "Failed flushing trace."; + } + return res == 0; } static jboolean android_os_Debug_dumpJavaBacktraceToFileTimeout(JNIEnv* env, jobject clazz,