Skip to content

GitLab

T
tangled
Commit 13257563 authored by Peter Kotwicz's avatar Peter Kotwicz Committed by Chromium LUCI CQ
Browse files
Options

Android: Include os.Handler stack for StrictMode violations 

If a strict mode violation occurs in an android.app.Service, the stack
sent to StrictMode.OnThreadViolationListener#onThreadViolation() only
includes stack frames which occurred in android.app.Service. Stack frames
for the call into the android.app.Service are truncated.

This is problematic when ignoring strict mode violations such as those
from android.content.ContentResolver#setSyncAutomatically where the
strict mode violation occurs inside the service called by
ContentResolver#setSyncAutomatically().

This CL switches to using Thread.currentThread().getStackTrace() to
obtain the non-truncated stack trace on Android P+. The CL also switches
to using reflection in order to get a callback for the strict mode
violation on the thread which triggered the strict mode violation.

BUG=1166997

Change-Id: Iab68f1e849b59725082122f93a8a877c27a89717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2637267
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#845076}
parent d0f7494a
Hide whitespace changes
Inline Side-by-side
Showing with 43 additions and 81 deletions
components/strictmode/android/BUILD.gn
View file @ 13257563
......@@ -10,7 +10,6 @@ android_library("java") {
"java/src/org/chromium/components/strictmode/ReflectiveThreadStrictModeInterceptor.java",
"java/src/org/chromium/components/strictmode/StrictModePolicyViolation.java",
"java/src/org/chromium/components/strictmode/ThreadStrictModeInterceptor.java",
"java/src/org/chromium/components/strictmode/ThreadStrictModeInterceptorP.java",
"java/src/org/chromium/components/strictmode/Violation.java",
]
deps = [
......
components/strictmode/android/java/src/org/chromium/components/strictmode/ReflectiveThreadStrictModeInterceptor.java
View file @ 13257563
......@@ -4,9 +4,14 @@
package org.chromium.components.strictmode;
import android.annotation.TargetApi;
import android.app.ApplicationErrorReport;
import android.os.Build;
import android.os.StrictMode;
import android.os.StrictMode.ThreadPolicy;
import android.os.strictmode.DiskReadViolation;
import android.os.strictmode.DiskWriteViolation;
import android.os.strictmode.ResourceMismatchViolation;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
......@@ -58,6 +63,10 @@ final class ReflectiveThreadStrictModeInterceptor implements ThreadStrictModeInt
@Override
public void install(ThreadPolicy detectors) {
// Use reflection on Android P despite the existence of
// StrictMode.OnThreadViolationListener because the listener receives a stack
// trace with stack frames prior to android.os.Handler calls stripped out.
interceptWithReflection();
StrictMode.setThreadPolicy(new ThreadPolicy.Builder(detectors).penaltyLog().build());
}
......@@ -98,14 +107,27 @@ final class ReflectiveThreadStrictModeInterceptor implements ThreadStrictModeInt
}
/** @param o {@code android.os.StrictMode.ViolationInfo} */
@SuppressWarnings({"unchecked", "PrivateApi"})
private static int getViolationType(Object o) {
@SuppressWarnings({"unchecked", "DiscouragedPrivateApi", "PrivateApi"})
private int getViolationType(Object violationInfo) {
try {
Class<?> violationInfo = Class.forName("android.os.StrictMode$ViolationInfo");
Field crashInfoField = violationInfo.getDeclaredField("crashInfo");
Class<?> violationInfoClass = Class.forName("android.os.StrictMode$ViolationInfo");
if (Build.VERSION.SDK_INT == 28) {
Method getViolationBitMethod =
violationInfoClass.getDeclaredMethod("getViolationBit");
getViolationBitMethod.setAccessible(true);
int violationType = (Integer) getViolationBitMethod.invoke(violationInfo);
return violationType & Violation.DETECT_ALL_KNOWN;
} else if (Build.VERSION.SDK_INT >= 29) {
Method getViolationClassMethod =
violationInfoClass.getDeclaredMethod("getViolationClass");
getViolationClassMethod.setAccessible(true);
return computeViolationTypeAndroid10(
(Class<?>) getViolationClassMethod.invoke(violationInfo));
}
Field crashInfoField = violationInfoClass.getDeclaredField("crashInfo");
crashInfoField.setAccessible(true);
ApplicationErrorReport.CrashInfo crashInfo =
(ApplicationErrorReport.CrashInfo) crashInfoField.get(o);
(ApplicationErrorReport.CrashInfo) crashInfoField.get(violationInfo);
Method parseViolationFromMessage =
StrictMode.class.getDeclaredMethod("parseViolationFromMessage", String.class);
parseViolationFromMessage.setAccessible(true);
......@@ -117,4 +139,19 @@ final class ReflectiveThreadStrictModeInterceptor implements ThreadStrictModeInt
return Violation.DETECT_UNKNOWN;
}
}
/**
* Computes the violation type based on the class of the passed-in violation.
*/
@TargetApi(29)
private static int computeViolationTypeAndroid10(Class<?> violationClass) {
if (DiskReadViolation.class.isAssignableFrom(violationClass)) {
return Violation.DETECT_DISK_READ;
} else if (DiskWriteViolation.class.isAssignableFrom(violationClass)) {
return Violation.DETECT_DISK_WRITE;
} else if (ResourceMismatchViolation.class.isAssignableFrom(violationClass)) {
return Violation.DETECT_RESOURCE_MISMATCH;
}
return Violation.DETECT_UNKNOWN;
}
}
components/strictmode/android/java/src/org/chromium/components/strictmode/ThreadStrictModeInterceptor.java
View file @ 13257563
......@@ -4,7 +4,6 @@
package org.chromium.components.strictmode;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.os.StrictMode.ThreadPolicy;
......@@ -181,11 +180,7 @@ public interface ThreadStrictModeInterceptor {
/** Make immutable. */
public ThreadStrictModeInterceptor build() {
if (Build.VERSION.SDK_INT >= 28) {
return new ThreadStrictModeInterceptorP(mWhitelistEntries, mCustomPenalty);
} else {
return new ReflectiveThreadStrictModeInterceptor(mWhitelistEntries, mCustomPenalty);
}
return new ReflectiveThreadStrictModeInterceptor(mWhitelistEntries, mCustomPenalty);
}
}
}
components/strictmode/android/java/src/org/chromium/components/strictmode/ThreadStrictModeInterceptorP.java deleted 100644 → 0
View file @ d0f7494a
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.components.strictmode;
import android.annotation.TargetApi;
import android.os.StrictMode;
import android.os.StrictMode.ThreadPolicy;
import android.os.strictmode.DiskReadViolation;
import android.os.strictmode.DiskWriteViolation;
import android.os.strictmode.ResourceMismatchViolation;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.chromium.base.Consumer;
import org.chromium.base.Function;
import java.util.List;
/**
* Android P+ {@ThreadStrictModeInterceptor} implementation.
*/
@TargetApi(28)
final class ThreadStrictModeInterceptorP implements ThreadStrictModeInterceptor {
@NonNull
private final List<Function<Violation, Integer>> mWhitelistEntries;
@Nullable
private final Consumer mCustomPenalty;
ThreadStrictModeInterceptorP(@NonNull List<Function<Violation, Integer>> whitelistEntries,
@Nullable Consumer customPenalty) {
mWhitelistEntries = whitelistEntries;
mCustomPenalty = customPenalty;
}
@Override
@TargetApi(28)
public void install(ThreadPolicy threadPolicy) {
StrictMode.OnThreadViolationListener listener = v -> {
handleThreadViolation(new Violation(computeType(v), v.getStackTrace()));
};
ThreadPolicy.Builder builder = new ThreadPolicy.Builder(threadPolicy);
builder.penaltyListener(Runnable::run, listener);
StrictMode.setThreadPolicy(builder.build());
}
private void handleThreadViolation(Violation violation) {
if (violation.isInWhitelist(mWhitelistEntries)) {
return;
}
if (mCustomPenalty != null) {
mCustomPenalty.accept(violation);
}
}
private static int computeType(android.os.strictmode.Violation violation) {
if (violation instanceof DiskReadViolation) {
return Violation.DETECT_DISK_READ;
} else if (violation instanceof DiskWriteViolation) {
return Violation.DETECT_DISK_WRITE;
} else if (violation instanceof ResourceMismatchViolation) {
return Violation.DETECT_RESOURCE_MISMATCH;
} else {
return Violation.DETECT_UNKNOWN;
}
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment