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

User Class.forName instead of ServiceLoader to instantiate Dispatchers.Main on Android #1572

Merged
merged 2 commits into from
Dec 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ private val handlers: List<CoroutineExceptionHandler> = ServiceLoader.load(
CoroutineExceptionHandler::class.java.classLoader
).iterator().asSequence().toList()


internal actual fun handleCoroutineExceptionImpl(context: CoroutineContext, exception: Throwable) {
// use additional extension handlers
for (handler in handlers) {
Expand Down
60 changes: 59 additions & 1 deletion kotlinx-coroutines-core/jvm/src/internal/FastServiceLoader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import java.net.*
import java.util.*
import java.util.jar.*
import java.util.zip.*
import kotlin.collections.ArrayList

/**
* Don't use JvmField here to enable R8 optimizations via "assumenosideeffects"
*/
internal val ANDROID_DETECTED = runCatching { Class.forName("android.os.Build") }.isSuccess

/**
* A simplified version of [ServiceLoader].
Expand All @@ -20,7 +26,59 @@ import java.util.zip.*
internal object FastServiceLoader {
private const val PREFIX: String = "META-INF/services/"

internal fun <S> load(service: Class<S>, loader: ClassLoader): List<S> {
/**
* This method attempts to load [MainDispatcherFactory] in Android-friendly way.
*
* If we are not on Android, this method fallbacks to a regular service loading,
* else we attempt to do `Class.forName` lookup for
* `AndroidDispatcherFactory` and `TestMainDispatcherFactory`.
* If lookups are successful, we return resultinAg instances because we know that
* `MainDispatcherFactory` API is internal and this is the only possible classes of `MainDispatcherFactory` Service on Android.
*
* Such intricate dance is required to avoid calls to `ServiceLoader.load` for multiple reasons:
* 1) It eliminates disk lookup on potentially slow devices on the Main thread.
* 2) Various Android toolchain versions by various vendors don't tend to handle ServiceLoader calls properly.
* Sometimes META-INF is removed from the resulting APK, sometimes class names are mangled, etc.
* While it is not the problem of `kotlinx.coroutines`, it significantly worsens user experience, thus we are workarounding it.
* Examples of such issues are #932, #1072, #1557, #1567
*
* We also use SL for [CoroutineExceptionHandler], but we do not experience the same problems and CEH is a public API
* that may already be injected vis SL, so we are not using the same technique for it.
*/
internal fun loadMainDispatcherFactory(): List<MainDispatcherFactory> {
val clz = MainDispatcherFactory::class.java
if (!ANDROID_DETECTED) {
return load(clz, clz.classLoader)
}

return try {
val result = ArrayList<MainDispatcherFactory>(2)
createInstanceOf(clz, "kotlinx.coroutines.android.AndroidDispatcherFactory")?.apply { result.add(this) }
createInstanceOf(clz, "kotlinx.coroutines.test.internal.TestMainDispatcherFactory")?.apply { result.add(this) }
result
} catch (e: Throwable) {
// Fallback to the regular SL in case of any unexpected exception
load(clz, clz.classLoader)
}
}

/*
* This method is inline to have a direct Class.forName("string literal") in the byte code to avoid weird interactions with ProGuard/R8.
*/
@Suppress("NOTHING_TO_INLINE")
private inline fun createInstanceOf(
baseClass: Class<MainDispatcherFactory>,
serviceClass: String
): MainDispatcherFactory? {
return try {
val clz = Class.forName(serviceClass, true, baseClass.classLoader)
qwwdfsad marked this conversation as resolved.
Show resolved Hide resolved
baseClass.cast(clz.getDeclaredConstructor().newInstance())
} catch (e: ClassNotFoundException) { // Do not fail if TestMainDispatcherFactory is not found
null
}
}

private fun <S> load(service: Class<S>, loader: ClassLoader): List<S> {
return try {
loadProviders(service, loader)
} catch (e: Throwable) {
Expand Down
10 changes: 4 additions & 6 deletions kotlinx-coroutines-core/jvm/src/internal/MainDispatchers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ internal object MainDispatcherLoader {
private fun loadMainDispatcher(): MainCoroutineDispatcher {
return try {
val factories = if (FAST_SERVICE_LOADER_ENABLED) {
MainDispatcherFactory::class.java.let { clz ->
FastServiceLoader.load(clz, clz.classLoader)
}
FastServiceLoader.loadMainDispatcherFactory()
} else {
//We are explicitly using the
//`ServiceLoader.load(MyClass::class.java, MyClass::class.java.classLoader).iterator()`
//form of the ServiceLoader call to enable R8 optimization when compiled on Android.
// We are explicitly using the
// `ServiceLoader.load(MyClass::class.java, MyClass::class.java.classLoader).iterator()`
// form of the ServiceLoader call to enable R8 optimization when compiled on Android.
ServiceLoader.load(
MainDispatcherFactory::class.java,
MainDispatcherFactory::class.java.classLoader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@
# this results in direct instantiation when loading Dispatchers.Main
-assumenosideeffects class kotlinx.coroutines.internal.MainDispatcherLoader {
boolean FAST_SERVICE_LOADER_ENABLED return false;
}

-assumenosideeffects class kotlinx.coroutines.internal.FastServiceLoader {
boolean ANDROID_DETECTED return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@
# - META-INF/proguard/coroutines.pro

-keep class kotlinx.coroutines.android.AndroidDispatcherFactory {*;}

-assumenosideeffects class kotlinx.coroutines.internal.FastServiceLoader {
boolean ANDROID_DETECTED return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class R8ServiceLoaderOptimizationTest : TestBase() {
}

@Test
@Ignore
fun noOptimRulesMatch() {
val paths = listOf(
"META-INF/com.android.tools/proguard/coroutines.pro",
Expand Down