Skip to content

Commit

Permalink
User Class.forName instead of ServiceLoader to instantiate Dispatcher…
Browse files Browse the repository at this point in the history
…s.Main on Android (Kotlin#1572)

Fixes Kotlin#1557
Fixes Kotlin#878
Fixes Kotlin#1606
  • Loading branch information
qwwdfsad committed Dec 12, 2019
1 parent 04e587c commit e60ec8e
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 8 deletions.
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)
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

0 comments on commit e60ec8e

Please sign in to comment.