From 9273c6326ea78d294d56b27fbd5363b7d7b5e79b Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Thu, 29 Apr 2021 14:07:34 -0700 Subject: [PATCH] FIR IDE: workaround deadlock when resolving class members When listing members in a class, we need to resolve super types. But if a super type is defined in another file, this file may not have been resolved. In this case, listing the members would require resolving the unresolved file, which can only be done in a write lock. Before this change, KtFirScopeProvider only acquire the read lock, which then causes read/write deadlock when the logic tries to further resolve super types defined in other files. This change makes KtFirScopeProvider acquire a write lock in the beginning so resolving can happen correctly later. --- .../low/level/api/api/LowLevelFirApiFacade.kt | 6 +++++- .../api/fir/components/KtFirScopeProvider.kt | 11 +++-------- .../api/fir/utils/FirRefWithValidityCheck.kt | 18 +++++++++++++++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/idea/idea-frontend-fir/idea-fir-low-level-api/src/org/jetbrains/kotlin/idea/fir/low/level/api/api/LowLevelFirApiFacade.kt b/idea/idea-frontend-fir/idea-fir-low-level-api/src/org/jetbrains/kotlin/idea/fir/low/level/api/api/LowLevelFirApiFacade.kt index 61492edbce2..0ab94cb0e8a 100644 --- a/idea/idea-frontend-fir/idea-fir-low-level-api/src/org/jetbrains/kotlin/idea/fir/low/level/api/api/LowLevelFirApiFacade.kt +++ b/idea/idea-frontend-fir/idea-fir-low-level-api/src/org/jetbrains/kotlin/idea/fir/low/level/api/api/LowLevelFirApiFacade.kt @@ -109,8 +109,12 @@ fun D.withFirDeclaration( */ fun D.withFirDeclarationInWriteLock( resolveState: FirModuleResolveState, + phase: FirResolvePhase = FirResolvePhase.RAW_FIR, action: (D) -> R, -): R = resolveState.withLock(this, DeclarationLockType.WRITE_LOCK, action) +): R { + resolvedFirToPhase(phase, resolveState) + return resolveState.withLock(this, DeclarationLockType.WRITE_LOCK, action) +} /** * Returns a list of Diagnostics compiler finds for given [KtElement] diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirScopeProvider.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirScopeProvider.kt index 035ebc4b7cc..f79afc12b8e 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirScopeProvider.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirScopeProvider.kt @@ -17,7 +17,6 @@ import org.jetbrains.kotlin.fir.scopes.* import org.jetbrains.kotlin.fir.scopes.impl.* import org.jetbrains.kotlin.idea.fir.low.level.api.api.FirModuleResolveState import org.jetbrains.kotlin.idea.fir.low.level.api.api.getTowerDataContextUnsafe -import org.jetbrains.kotlin.idea.frontend.api.tokens.ValidityToken import org.jetbrains.kotlin.idea.frontend.api.ValidityTokenOwner import org.jetbrains.kotlin.idea.frontend.api.components.KtScopeContext import org.jetbrains.kotlin.idea.frontend.api.components.KtScopeProvider @@ -25,18 +24,14 @@ import org.jetbrains.kotlin.idea.frontend.api.fir.KtFirAnalysisSession import org.jetbrains.kotlin.idea.frontend.api.fir.KtSymbolByFirBuilder import org.jetbrains.kotlin.idea.frontend.api.fir.scopes.* import org.jetbrains.kotlin.idea.frontend.api.fir.symbols.* -import org.jetbrains.kotlin.idea.frontend.api.fir.symbols.KtFirAnonymousObjectSymbol -import org.jetbrains.kotlin.idea.frontend.api.fir.symbols.KtFirEnumEntrySymbol -import org.jetbrains.kotlin.idea.frontend.api.fir.symbols.KtFirFileSymbol -import org.jetbrains.kotlin.idea.frontend.api.fir.symbols.KtFirNamedClassOrObjectSymbol import org.jetbrains.kotlin.idea.frontend.api.fir.types.KtFirType import org.jetbrains.kotlin.idea.frontend.api.fir.utils.weakRef import org.jetbrains.kotlin.idea.frontend.api.scopes.* -import org.jetbrains.kotlin.idea.frontend.api.symbols.KtClassOrObjectSymbol import org.jetbrains.kotlin.idea.frontend.api.symbols.KtFileSymbol import org.jetbrains.kotlin.idea.frontend.api.symbols.KtPackageSymbol import org.jetbrains.kotlin.idea.frontend.api.symbols.markers.KtSymbolWithDeclarations import org.jetbrains.kotlin.idea.frontend.api.symbols.markers.KtSymbolWithMembers +import org.jetbrains.kotlin.idea.frontend.api.tokens.ValidityToken import org.jetbrains.kotlin.idea.frontend.api.types.KtType import org.jetbrains.kotlin.idea.frontend.api.withValidityAssertion import org.jetbrains.kotlin.psi.KtElement @@ -61,8 +56,8 @@ internal class KtFirScopeProvider( private val packageMemberScopeCache = IdentityHashMap() private inline fun KtSymbolWithMembers.withFirForScope(crossinline body: (FirClass<*>) -> T): T? = when (this) { - is KtFirNamedClassOrObjectSymbol -> firRef.withFir(FirResolvePhase.TYPES, body) - is KtFirAnonymousObjectSymbol -> firRef.withFir(FirResolvePhase.TYPES, body) + is KtFirNamedClassOrObjectSymbol -> firRef.withFirWithPossibleResolveInside(FirResolvePhase.TYPES, body) + is KtFirAnonymousObjectSymbol -> firRef.withFirWithPossibleResolveInside(FirResolvePhase.TYPES, body) is KtFirEnumEntrySymbol -> firRef.withFir(FirResolvePhase.IMPLICIT_TYPES_BODY_RESOLVE) { val initializer = it.initializer check(initializer is FirAnonymousObject) diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/utils/FirRefWithValidityCheck.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/utils/FirRefWithValidityCheck.kt index 7b89a422b2f..700b18b40a4 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/utils/FirRefWithValidityCheck.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/utils/FirRefWithValidityCheck.kt @@ -11,8 +11,8 @@ import org.jetbrains.kotlin.fir.declarations.FirResolvePhase import org.jetbrains.kotlin.idea.fir.low.level.api.api.FirModuleResolveState import org.jetbrains.kotlin.idea.fir.low.level.api.api.withFirDeclaration import org.jetbrains.kotlin.idea.fir.low.level.api.api.withFirDeclarationInWriteLock -import org.jetbrains.kotlin.idea.frontend.api.tokens.ValidityToken import org.jetbrains.kotlin.idea.frontend.api.ValidityTokenOwner +import org.jetbrains.kotlin.idea.frontend.api.tokens.ValidityToken import org.jetbrains.kotlin.idea.frontend.api.tokens.assertIsValidAndAccessible import java.lang.ref.WeakReference @@ -58,13 +58,25 @@ internal class FirRefWithValidityCheck(fir: D, resolveSt * Runs [action] with fir element with write action hold * Consider using this then [action] may call some resolve */ - inline fun withFirWithPossibleResolveInside(crossinline action: (fir: D) -> R): R { + inline fun withFirWithPossibleResolveInside( + phase: FirResolvePhase = FirResolvePhase.RAW_FIR, + crossinline action: (fir: D) -> R + ): R { token.assertIsValidAndAccessible() val fir = firWeakRef.get() ?: throw EntityWasGarbageCollectedException("FirElement") val resolveState = resolveStateWeakRef.get() ?: throw EntityWasGarbageCollectedException("FirModuleResolveState") - return fir.withFirDeclarationInWriteLock(resolveState) { action(it) } + return when (phase) { + FirResolvePhase.BODY_RESOLVE -> { + /* + The BODY_RESOLVE phase is the maximum possible phase we can resolve our declaration to + So there is not need to run whole `action` under write lock + */ + action(fir.withFirDeclarationInWriteLock(resolveState, phase) { it }) + } + else -> fir.withFirDeclarationInWriteLock(resolveState, phase) { action(it) } + } } val resolveState