Load services both from regular jars and files, filter duplicates

Author: qwwdfsad

kotlinx-coroutines-core/build.gradle

@@ -46,7 +46,6 @@ kotlin.sourceSets {
     jvmTest.dependencies {
         api "com.devexperts.lincheck:lincheck:$lincheck_version"
         api "com.esotericsoftware:kryo:4.0.0"
-
         implementation project (":android-unit-tests")
     }
 }

kotlinx-coroutines-core/jvm/src/internal/FastServiceLoader.kt

@@ -1,15 +1,15 @@
 package kotlinx.coroutines.internal
 
-import java.util.*
 import java.io.*
 import java.net.*
+import java.util.*
 import java.util.jar.*
 import java.util.zip.*
 
 /**
  * Name of the boolean property that enables using of [FastServiceLoader].
  */
-private const val FAST_SERVICE_LOADER_PROPERTY_NAME = "kotlinx.coroutines.verify.service.loader"
+private const val FAST_SERVICE_LOADER_PROPERTY_NAME = "kotlinx.coroutines.fast.service.loader"
 
 /**
  * A simplified version of [ServiceLoader].
@@ -22,12 +22,10 @@ private const val FAST_SERVICE_LOADER_PROPERTY_NAME = "kotlinx.coroutines.verify
  *
  * If any error occurs during loading, it fallbacks to [ServiceLoader], mostly to prevent R8 issues.
  */
-
 internal object FastServiceLoader {
     private const val PREFIX: String = "META-INF/services/"
 
-    @JvmField
-    internal val FAST_SERVICE_LOADER_ENABLED = systemProp(FAST_SERVICE_LOADER_PROPERTY_NAME, true)
+    private val FAST_SERVICE_LOADER_ENABLED = systemProp(FAST_SERVICE_LOADER_PROPERTY_NAME, true)
 
     internal fun <S> load(service: Class<S>, loader: ClassLoader): List<S> {
         if (!FAST_SERVICE_LOADER_ENABLED) {
@@ -41,16 +39,14 @@ internal object FastServiceLoader {
         }
     }
 
+    // Visible for tests
     internal fun <S> loadProviders(service: Class<S>, loader: ClassLoader): List<S> {
         val fullServiceName = PREFIX + service.name
-        val urls = loader.getResources(fullServiceName).toList()
-        val providers = mutableListOf<S>()
-        urls.forEach {
-            val providerNames = parse(it)
-            providers.addAll(providerNames.map { getProviderInstance(it, loader, service) })
-        }
+        // Filter out situations when both JAR and regular files are in the classpath (e.g. IDEA)
+        val urls = loader.getResources(fullServiceName)
+        val providers = urls.toList().flatMap { parse(it) }.toSet()
         require(providers.isNotEmpty()) { "No providers were loaded with FastServiceLoader" }
-        return providers
+        return providers.map { getProviderInstance(it, loader, service) }
     }
 
     private fun <S> getProviderInstance(name: String, loader: ClassLoader, service: Class<S>): S {
@@ -60,16 +56,22 @@ internal object FastServiceLoader {
     }
 
     private fun parse(url: URL): List<String> {
-        val string = url.toString()
-        return if (string.startsWith("jar")) {
-            val pathToJar = string.substringAfter("jar:file:").substringBefore('!')
-            val entry = string.substringAfter("!/")
+        val path = url.toString()
+        // Fast-path for JARs
+        if (path.startsWith("jar")) {
+            val pathToJar = path.substringAfter("jar:file:").substringBefore('!')
+            val entry = path.substringAfter("!/")
+            // mind the verify = false flag!
             (JarFile(pathToJar, false) as Closeable).use { file ->
-                BufferedReader(InputStreamReader((file as JarFile).getInputStream(ZipEntry(entry)),"UTF-8")).use { r ->
-                    parseFile(r)
+                BufferedReader(InputStreamReader((file as JarFile).getInputStream(ZipEntry(entry)), "UTF-8")).use { r ->
+                    return parseFile(r)
                 }
             }
-        } else emptyList()
+        }
+        // Regular path for everything elese
+        return BufferedReader(InputStreamReader(url.openStream())).use { reader ->
+            parseFile(reader)
+        }
     }
 
     private fun parseFile(r: BufferedReader): List<String> {

kotlinx-coroutines-core/jvm/test/CommonPoolTest.kt

@@ -39,7 +39,6 @@ class CommonPoolTest {
         val fjp1: ExecutorService = createFJP(1, fjpCtor, dwtfCtor) ?: return
         assertTrue(CommonPool.isGoodCommonPool(fjpClass, fjp1))
         fjp1.shutdown()
-        println("CommonPool.isGoodCommonPool test passed")
     }
 
     private fun createFJP(

kotlinx-coroutines-core/jvm/test/internal/ServiceLoaderTest.kt renamed to kotlinx-coroutines-core/jvm/test/internal/FastServiceLoaderTest.kt

@@ -2,19 +2,20 @@ package kotlinx.coroutines.internal
 
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Delay
-import kotlin.test.Test
+import kotlin.test.*
 
-class ServiceLoaderTest {
+class FastServiceLoaderTest {
     @Test
     fun testLoadingSameModuleService() {
         val providers = Delay::class.java.let { FastServiceLoader.loadProviders(it, it.classLoader) }
-        assert(providers.size == 1 && providers[0].javaClass.name == "kotlinx.coroutines.android.DelayImpl")
+        assertEquals(1, providers.size)
+        assertEquals("kotlinx.coroutines.android.DelayImpl", providers[0].javaClass.name)
     }
 
     @Test
     fun testCrossModuleService() {
         val providers = CoroutineScope::class.java.let { FastServiceLoader.loadProviders(it, it.classLoader) }
-        assert(providers.size == 3)
+        assertEquals(3, providers.size)
         val className = "kotlinx.coroutines.android.EmptyCoroutineScopeImpl"
         for (i in 1 .. 3) {
             assert(providers[i - 1].javaClass.name == "$className$i")

ui/kotlinx-coroutines-android/android-unit-tests/src/DelayImpl.kt

@@ -2,9 +2,9 @@ package kotlinx.coroutines.android
 
 import kotlinx.coroutines.*
 
-@InternalCoroutinesApi
+// Class for testing service loader
 class DelayImpl : Delay {
     override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation<Unit>) {
         continuation.cancel()
     }
-}
+}

ui/kotlinx-coroutines-android/android-unit-tests/src/EmptyCoroutineScopeImpl.kt

@@ -4,6 +4,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlin.coroutines.CoroutineContext
 import kotlin.coroutines.EmptyCoroutineContext
 
+// Classes for testing service loader
 internal class EmptyCoroutineScopeImpl1 : CoroutineScope {
     override val coroutineContext: CoroutineContext
         get() = EmptyCoroutineContext