From 25f2c513f6f4492b23d62579298f5676038e74eb Mon Sep 17 00:00:00 2001 From: Dmitrii Gridin Date: Mon, 8 May 2023 15:00:27 +0200 Subject: [PATCH] [LL FIR] drop global phase lock for super types phase ^KT-56551 --- .../api/fir/file/builder/LLFirLockProvider.kt | 2 - .../LLFirSupertypeLazyResolver.kt | 44 ++++++++++--------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt index d81ff02b7cf..36a0053dca6 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/file/builder/LLFirLockProvider.kt @@ -27,7 +27,6 @@ import org.jetbrains.kotlin.fir.declarations.FirFile internal class LLFirLockProvider(private val checker: LLFirLazyResolveContractChecker) { private val globalLock = ReentrantLock() - private val superTypesLock = ReentrantLock() private val implicitTypesLock = ReentrantLock() private val statusLock = ReentrantLock() private val compilerAnnotationsLock = ReentrantLock() @@ -55,7 +54,6 @@ internal class LLFirLockProvider(private val checker: LLFirLazyResolveContractCh val lock = when (phase) { FirResolvePhase.IMPLICIT_TYPES_BODY_RESOLVE -> implicitTypesLock FirResolvePhase.STATUS -> statusLock - FirResolvePhase.SUPER_TYPES -> superTypesLock FirResolvePhase.COMPILER_REQUIRED_ANNOTATIONS -> compilerAnnotationsLock else -> null } diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/transformers/LLFirSupertypeLazyResolver.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/transformers/LLFirSupertypeLazyResolver.kt index 6e418532fdb..f1d1bcd8c9c 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/transformers/LLFirSupertypeLazyResolver.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/transformers/LLFirSupertypeLazyResolver.kt @@ -110,7 +110,12 @@ private class LLFirSuperTypeTargetResolver( when (target) { is FirRegularClass -> performResolve( declaration = target, - resolver = { target.resolveSupertypeRefs() }, + superTypeRefsForTransformation = { + // We should create a copy of the original collection + // to avoid [ConcurrentModificationException] during another thread publication + ArrayList(target.superTypeRefs) + }, + resolver = { supertypeResolver.resolveSpecificClassLikeSupertypes(target, it) }, superTypeUpdater = { target.replaceSuperTypeRefs(it) session.platformSupertypeUpdater?.updateSupertypesIfNeeded(target, scopeSession) @@ -118,7 +123,8 @@ private class LLFirSuperTypeTargetResolver( ) is FirTypeAlias -> performResolve( declaration = target, - resolver = { target.resolveExpandedTypeRef() }, + superTypeRefsForTransformation = { target.expandedTypeRef }, + resolver = { supertypeResolver.resolveTypeAliasSupertype(target, it, resolveRecursively = false) }, superTypeUpdater = { target.replaceExpandedTypeRef(it.single()) }, ) else -> performCustomResolveUnderLock(target) { @@ -129,24 +135,13 @@ private class LLFirSuperTypeTargetResolver( return true } - private fun FirRegularClass.resolveSupertypeRefs(): List { - val superTypeRefs = superTypeRefs - /** - * TODO: KT-56551 this is safe, because we have a global phase lock - * Without such lock we should make a copy of [superTypeRefs] under a lock or - * FirRegularClassImpl should have "var superTypeRefs" + assign instead of "val superTypeRefs" + mutation - */ - return supertypeResolver.resolveSpecificClassLikeSupertypes(this, superTypeRefs) - } - - private fun FirTypeAlias.resolveExpandedTypeRef(): List { - val expandedTypeRef = expandedTypeRef - return supertypeResolver.resolveTypeAliasSupertype(this, expandedTypeRef, resolveRecursively = false) - } - - private inline fun performResolve( + /** + * [superTypeRefsForTransformation] will be executed under [declaration] lock + */ + private inline fun performResolve( declaration: T, - resolver: () -> List, + superTypeRefsForTransformation: () -> S, + resolver: (S) -> List, crossinline superTypeUpdater: (List) -> Unit, ) { // To avoid redundant work, because a publication won't be executed @@ -154,8 +149,17 @@ private class LLFirSuperTypeTargetResolver( declaration.lazyResolveToPhase(resolverPhase.previous) + var superTypeRefs: S? = null + withReadLock(declaration) { + superTypeRefs = superTypeRefsForTransformation() + } + + // "null" means that some other thread is already resolved [declaration] to [resolverPhase] + if (superTypeRefs == null) return + // 1. Resolve declaration super type refs - val resolvedSuperTypeRefs = resolver() + @Suppress("UNCHECKED_CAST") + val resolvedSuperTypeRefs = resolver(superTypeRefs as S) // 2. Resolve super declarations val status = supertypeComputationSession.getSupertypesComputationStatus(declaration)