For 17920: add ThreadPenaltyDeathWithIgnoresListener, tests, helpers.
Michael Comella
3 years ago
committed by
Michael Comella
parent
e9e067615a
commit
a86b4ef1bd
@ -0,0 +1,73 @@
|
||||
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||
|
||||
package org.mozilla.fenix.perf
|
||||
|
||||
import android.os.Build
|
||||
import android.os.StrictMode
|
||||
import android.os.strictmode.Violation
|
||||
import androidx.annotation.RequiresApi
|
||||
import mozilla.components.support.base.log.logger.Logger
|
||||
import org.mozilla.fenix.utils.ManufacturerCodes
|
||||
|
||||
private const val FCQN_EDM_STORAGE_PROVIDER_BASE = "com.android.server.enterprise.storage.EdmStorageProviderBase"
|
||||
|
||||
/**
|
||||
* A [StrictMode.OnThreadViolationListener] that recreates
|
||||
* [StrictMode.ThreadPolicy.Builder.penaltyDeath] but will ignore some violations. For example,
|
||||
* sometimes OEMs will add code that violates StrictMode so we can ignore them here instead of
|
||||
* cluttering up our code with resetAfter.
|
||||
*
|
||||
* This class can only be used with Android P+ so we'd have to implement workarounds if the
|
||||
* violations we want to ignore affect older devices.
|
||||
*/
|
||||
@RequiresApi(Build.VERSION_CODES.P)
|
||||
class ThreadPenaltyDeathWithIgnoresListener(
|
||||
private val logger: Logger = Performance.logger
|
||||
) : StrictMode.OnThreadViolationListener {
|
||||
|
||||
override fun onThreadViolation(violation: Violation?) {
|
||||
if (violation == null) return
|
||||
|
||||
// Unfortunately, this method gets called many (~5+) times with the same violation so we end
|
||||
// up logging/throwing redundantly.
|
||||
if (shouldViolationBeIgnored(violation)) {
|
||||
logger.debug("Ignoring StrictMode ThreadPolicy violation", violation)
|
||||
} else {
|
||||
penaltyDeath(violation)
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("TooGenericExceptionThrown") // we throw what StrictMode's penaltyDeath throws.
|
||||
private fun penaltyDeath(violation: Violation) {
|
||||
throw RuntimeException("StrictMode ThreadPolicy violation", violation)
|
||||
}
|
||||
|
||||
private fun shouldViolationBeIgnored(violation: Violation): Boolean =
|
||||
isSamsungLgEdmStorageProviderStartupViolation(violation)
|
||||
|
||||
private fun isSamsungLgEdmStorageProviderStartupViolation(violation: Violation): Boolean {
|
||||
// Root issue: https://github.com/mozilla-mobile/fenix/issues/17920
|
||||
//
|
||||
// This fix may address the issues seen in this bug:
|
||||
// https://github.com/mozilla-mobile/fenix/issues/15430
|
||||
// So we might be able to back out the changes made there. However, I don't have a device to
|
||||
// test so I didn't bother.
|
||||
//
|
||||
// This issue occurs on the Galaxy S10e, Galaxy A50, Note 10, and LG G7 FIT but not the S7:
|
||||
// I'm guessing it's just a problem on recent Samsungs and LGs so it's okay being in this P+
|
||||
// listener.
|
||||
if (!ManufacturerCodes.isSamsung && !ManufacturerCodes.isLG) {
|
||||
return false
|
||||
}
|
||||
|
||||
// To ignore this warning, we can inspect the stack trace. There are no parts of the
|
||||
// violation stack trace that are clearly unique to this violation but
|
||||
// EdmStorageProviderBase doesn't appear in Android code search so we match against it.
|
||||
// This class may be used in other violations that we're capable of fixing but this
|
||||
// code may ignore them. I think it's okay - we keep this code simple and if it was a serious
|
||||
// issue, we'd catch it on other manufacturers.
|
||||
return violation.stackTrace.any { it.className == FCQN_EDM_STORAGE_PROVIDER_BASE }
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,39 @@
|
||||
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||
|
||||
package org.mozilla.fenix.helpers
|
||||
|
||||
/**
|
||||
* A collection of test helper functions for manipulating stack traces.
|
||||
*/
|
||||
object StackTraces {
|
||||
|
||||
/**
|
||||
* Gets a stack trace from logcat output. To use this, you should remove the name of the
|
||||
* Exception or "Caused by" lines causing the problem and only use the stack trace lines below
|
||||
* it. See src/test/resources/EdmStorageProviderBaseLogcat.txt for an example.
|
||||
*/
|
||||
fun getStackTraceFromLogcat(logcatResourcePath: String): Array<StackTraceElement> {
|
||||
val logcat = javaClass.classLoader!!.getResource(logcatResourcePath).readText()
|
||||
val lines = logcat.split('\n').filter(String::isNotBlank)
|
||||
return lines.map(::logcatLineToStackTraceElement).toTypedArray()
|
||||
}
|
||||
|
||||
private fun logcatLineToStackTraceElement(line: String): StackTraceElement {
|
||||
// Expected format:
|
||||
// 02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1556)
|
||||
|
||||
// Expected: android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk
|
||||
val methodInfo = line.substringBefore('(').substringAfterLast(' ')
|
||||
val methodName = methodInfo.substringAfterLast('.')
|
||||
val declaringClass = methodInfo.substringBeforeLast('.')
|
||||
|
||||
// Expected: StrictMode.java:1556
|
||||
val fileInfo = line.substringAfter('(').substringBefore(')')
|
||||
val fileName = fileInfo.substringBefore(':')
|
||||
val lineNumber = fileInfo.substringAfter(':').toInt()
|
||||
|
||||
return StackTraceElement(declaringClass, methodName, fileName, lineNumber)
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,78 @@
|
||||
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||
|
||||
package org.mozilla.fenix.perf
|
||||
|
||||
import android.os.strictmode.Violation
|
||||
import io.mockk.MockKAnnotations
|
||||
import io.mockk.every
|
||||
import io.mockk.impl.annotations.MockK
|
||||
import io.mockk.impl.annotations.RelaxedMockK
|
||||
import io.mockk.mockkObject
|
||||
import io.mockk.unmockkAll
|
||||
import io.mockk.verify
|
||||
import mozilla.components.support.base.log.logger.Logger
|
||||
import org.junit.After
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.mozilla.fenix.helpers.StackTraces
|
||||
import org.mozilla.fenix.utils.ManufacturerCodes
|
||||
|
||||
class ThreadPenaltyDeathWithIgnoresListenerTest {
|
||||
|
||||
@RelaxedMockK private lateinit var logger: Logger
|
||||
private lateinit var listener: ThreadPenaltyDeathWithIgnoresListener
|
||||
@MockK private lateinit var violation: Violation
|
||||
private lateinit var stackTrace: Array<StackTraceElement>
|
||||
|
||||
@Before
|
||||
fun setUp() {
|
||||
MockKAnnotations.init(this)
|
||||
|
||||
listener = ThreadPenaltyDeathWithIgnoresListener(logger)
|
||||
|
||||
stackTrace = emptyArray()
|
||||
every { violation.stackTrace } answers { stackTrace }
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() {
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
@Test(expected = RuntimeException::class)
|
||||
fun `WHEN provided an arbitrary violation that does not conflict with ignores THEN we throw an exception`() {
|
||||
stackTrace = Exception().stackTrace
|
||||
listener.onThreadViolation(violation)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `GIVEN we're on a Samsung WHEN provided the EdmStorageProvider violation THEN it will be ignored and logged`() {
|
||||
mockkObject(ManufacturerCodes)
|
||||
every { ManufacturerCodes.isSamsung } returns true
|
||||
|
||||
every { violation.stackTrace } returns getEdmStorageProviderStackTrace()
|
||||
listener.onThreadViolation(violation)
|
||||
|
||||
verify { logger.debug("Ignoring StrictMode ThreadPolicy violation", violation) }
|
||||
}
|
||||
|
||||
@Test(expected = RuntimeException::class)
|
||||
fun `GIVEN we're not on a Samsung or LG WHEN provided the EdmStorageProvider violation THEN we throw an exception`() {
|
||||
mockkObject(ManufacturerCodes)
|
||||
every { ManufacturerCodes.isSamsung } returns false
|
||||
every { ManufacturerCodes.isLG } returns false
|
||||
|
||||
every { violation.stackTrace } returns getEdmStorageProviderStackTrace()
|
||||
listener.onThreadViolation(violation)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `WHEN violation is null THEN we don't throw an exception`() {
|
||||
listener.onThreadViolation(null)
|
||||
}
|
||||
|
||||
private fun getEdmStorageProviderStackTrace() =
|
||||
StackTraces.getStackTraceFromLogcat("EdmStorageProviderBaseLogcat.txt")
|
||||
}
|
||||
@ -0,0 +1,20 @@
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:1556)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteConnection.applyBlockGuardPolicy(SQLiteConnection.java:1524)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteConnection.executeForCursorWindow(SQLiteConnection.java:1193)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteSession.executeForCursorWindow(SQLiteSession.java:838)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteQuery.fillWindow(SQLiteQuery.java:62)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteCursor.fillWindow(SQLiteCursor.java:165)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.sqlite.SQLiteCursor.getCount(SQLiteCursor.java:152)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.AbstractCursor.moveToPosition(AbstractCursor.java:232)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.database.AbstractCursor.moveToFirst(AbstractCursor.java:271)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProviderBase.getValues(EdmStorageProviderBase.java:388)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProviderBase.getValuesList(EdmStorageProviderBase.java:1104)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.storage.EdmStorageProvider.getValuesList(EdmStorageProvider.java:300)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.application.ApplicationPolicy.getApplicationStateEnabledAsUser(ApplicationPolicy.java:3745)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.enterprise.application.ApplicationPolicy.getApplicationStateEnabledAsUser(ApplicationPolicy.java:3714)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.setEnabledSetting(PackageManagerService.java:26031)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.setComponentEnabledSetting(PackageManagerService.java:26007)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.content.pm.IPackageManager$Stub.onTransact(IPackageManager.java:3774)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at com.android.server.pm.PackageManagerService.onTransact(PackageManagerService.java:5145)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.Binder.execTransactInternal(Binder.java:1056)
|
||||
02-08 10:56:02.185 21990 21990 E AndroidRuntime: at android.os.Binder.execTransact(Binder.java:1029)
|
||||
Loading…
Reference in New Issue