From 77ba9a1bbbd166dad8dce4612ea4bd7989c67826 Mon Sep 17 00:00:00 2001 From: Ivan Gavrilovic Date: Sat, 27 Jun 2020 16:28:08 +0100 Subject: [PATCH] KT-34604: Fix race condition in KAPT KAPT was relying on clearing JarFileFactory to make sure annotation processing does not hold onto annotation processing classpath once done. Once KAPT switched to using Gradle workers, multiple KAPT runs were sharing the same class loader ie. the same version of JarFileFactory. Clearing the cache resulted in race condition, as some runs were unable to load processors from jars. This commit fixes this problem by avoiding the use of ServiceLoader which was causing the issue. Jars would be added to the cache, but they would never be removed. That's why JarFileFactory had to be clearned manually. By loading the processor names manually (simply exploring the classpath), no file handles leak. Fixes https://youtrack.jetbrains.com/issue/KT-34604 Test: verified against the test project from the bug --- .../kotlin/kapt3/base/ProcessorLoader.kt | 77 +++++++++++-------- .../jetbrains/kotlin/kapt3/Kapt3Extension.kt | 2 +- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/ProcessorLoader.kt b/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/ProcessorLoader.kt index 66179c8d57a..fbe1e682a5d 100644 --- a/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/ProcessorLoader.kt +++ b/plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/ProcessorLoader.kt @@ -14,11 +14,11 @@ import org.jetbrains.kotlin.kapt3.base.util.KaptLogger import org.jetbrains.kotlin.kapt3.base.util.info import java.io.Closeable import java.io.File -import java.lang.reflect.Field -import java.lang.reflect.Modifier +import java.io.InputStream import java.net.URLClassLoader -import java.util.* +import java.util.zip.ZipFile import javax.annotation.processing.Processor +import kotlin.collections.LinkedHashSet class LoadedProcessors(val processors: List, val classLoader: ClassLoader) @@ -26,8 +26,6 @@ open class ProcessorLoader(private val options: KaptOptions, private val logger: private var annotationProcessingClassLoader: URLClassLoader? = null fun loadProcessors(parentClassLoader: ClassLoader = ClassLoader.getSystemClassLoader()): LoadedProcessors { - clearJarURLCache() - val classpath = LinkedHashSet().apply { addAll(options.processingClasspath) if (options[KaptFlag.INCLUDE_COMPILE_CLASSPATH]) { @@ -42,7 +40,7 @@ open class ProcessorLoader(private val options: KaptOptions, private val logger: options.processors.mapNotNull { tryLoadProcessor(it, classLoader) } } else { logger.info("Need to discovery annotation processors in the AP classpath") - doLoadProcessors(classLoader) + doLoadProcessors(classpath, classLoader) } if (processors.isEmpty()) { @@ -76,8 +74,48 @@ open class ProcessorLoader(private val options: KaptOptions, private val logger: } } - open fun doLoadProcessors(classLoader: URLClassLoader): List { - return ServiceLoader.load(Processor::class.java, classLoader).toList() + open fun doLoadProcessors(classpath: LinkedHashSet, classLoader: URLClassLoader): List { + val processorNames = mutableSetOf() + + fun processSingleInput(input: InputStream) { + val lines = input.bufferedReader().lineSequence() + lines.forEach { line -> + val processedLine = line.substringBefore("#").trim() + if (processedLine.isNotEmpty()) { + processorNames.add(processedLine) + } + } + } + // Do not use ServiceLoader as it uses JarFileFactory cache which is not cleared + // properly. This may cause issues on Windows. + // Previously, JarFileFactory caches were manually cleaned, but that caused race conditions, + // as JarFileFactory was shared between concurrent runs in the same class loader. + // See https://youtrack.jetbrains.com/issue/KT-34604 for more details. Similar issue + // is also https://youtrack.jetbrains.com/issue/KT-22513. + val serviceFile = "META-INF/services/javax.annotation.processing.Processor" + for (file in classpath) { + when { + file.isDirectory -> { + file.resolve(serviceFile).takeIf { it.isFile }?.let { + processSingleInput(it.inputStream()) + } + } + file.isFile -> { + ZipFile(file).use { zipFile -> + zipFile.getEntry(serviceFile)?.let { zipEntry -> + zipFile.getInputStream(zipEntry).use { + processSingleInput(it) + } + } + } + } + else -> { + logger.info("$file cannot be used to locate $serviceFile file.") + } + } + } + + return processorNames.mapNotNull { tryLoadProcessor(it, classLoader) } } private fun tryLoadProcessor(fqName: String, classLoader: ClassLoader): Processor? { @@ -104,28 +142,5 @@ open class ProcessorLoader(private val options: KaptOptions, private val logger: override fun close() { annotationProcessingClassLoader?.close() - clearJarURLCache() - } -} - -// Copied from com.intellij.ide.ClassUtilCore -private fun clearJarURLCache() { - fun clearMap(cache: Field) { - cache.isAccessible = true - - if (!Modifier.isFinal(cache.modifiers)) { - cache.set(null, hashMapOf()) - } else { - val map = cache.get(null) as MutableMap<*, *> - map.clear() - } - } - - try { - val jarFileFactory = Class.forName("sun.net.www.protocol.jar.JarFileFactory") - - clearMap(jarFileFactory.getDeclaredField("fileCache")) - clearMap(jarFileFactory.getDeclaredField("urlCache")) - } catch (ignore: Exception) { } } \ No newline at end of file diff --git a/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Extension.kt b/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Extension.kt index 1c005ddbb03..2e00ae90d63 100644 --- a/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Extension.kt +++ b/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Extension.kt @@ -81,7 +81,7 @@ class ClasspathBasedKapt3Extension( override fun loadProcessors(): LoadedProcessors { val efficientProcessorLoader = object : ProcessorLoader(options, logger) { - override fun doLoadProcessors(classLoader: URLClassLoader): List { + override fun doLoadProcessors(classpath: LinkedHashSet, classLoader: URLClassLoader): List { return ServiceLoaderLite.loadImplementations(Processor::class.java, classLoader) } }