Skip to content

Commit

Permalink
Fix issue intellij-rust#9432 random false-positives caused by GC acti…
Browse files Browse the repository at this point in the history
…vity

Actually, the bug is a wrong (missing) implementation of CargoBasedCrate.equals/hashCode
  • Loading branch information
vlad20012 authored and yopox committed Feb 27, 2023
1 parent 70dc928 commit 40bbade
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 2 deletions.
22 changes: 22 additions & 0 deletions src/main/kotlin/org/rust/lang/core/crate/impl/CargoBasedCrate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import org.rust.lang.core.psi.rustFile
import org.rust.openapiext.fileId
import org.rust.openapiext.toPsiFile

/**
* [equals]/[hashCode] are based on [cargoTarget] and [id] fields
* because these fields equality guarantee other fields equality
*/
class CargoBasedCrate(
override var cargoProject: CargoProject,
override var cargoTarget: CargoWorkspace.Target,
Expand Down Expand Up @@ -72,6 +76,24 @@ class CargoBasedCrate(
override val normName: String get() = cargoTarget.normName

override fun toString(): String = "${cargoTarget.name}(${kind.name})"

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as CargoBasedCrate

if (cargoTarget != other.cargoTarget) return false
if (id != other.id) return false

return true
}

override fun hashCode(): Int {
var result = cargoTarget.hashCode()
result = 31 * result + (id ?: 0)
return result
}
}

// See https://github.com/rust-lang/cargo/blob/5a0c31d81/src/cargo/core/manifest.rs#L775
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ import com.intellij.openapi.util.SimpleModificationTracker
import com.intellij.openapi.vfs.VirtualFile
import com.intellij.openapi.vfs.VirtualFileManager
import com.intellij.openapi.vfs.VirtualFileWithId
import com.intellij.psi.util.CachedValue
import com.intellij.psi.util.CachedValueProvider
import com.intellij.util.CachedValueImpl
import com.intellij.util.containers.addIfNotNull
import gnu.trove.TIntObjectHashMap
import org.jetbrains.annotations.TestOnly
import org.rust.cargo.project.model.CargoProject
import org.rust.cargo.project.model.CargoProjectsService.CargoProjectsListener
import org.rust.cargo.project.model.CargoProjectsService.Companion.CARGO_PROJECTS_TOPIC
Expand All @@ -25,7 +28,6 @@ import org.rust.cargo.project.workspace.PackageOrigin
import org.rust.lang.core.crate.Crate
import org.rust.lang.core.crate.CrateGraphService
import org.rust.lang.core.crate.CratePersistentId
import org.rust.openapiext.CachedValueDelegate
import org.rust.openapiext.checkReadAccessAllowed
import org.rust.openapiext.isUnitTestMode
import org.rust.stdext.applyWithSymlink
Expand All @@ -43,11 +45,14 @@ class CrateGraphServiceImpl(val project: Project) : CrateGraphService {
})
}

private val crateGraph: CrateGraph by CachedValueDelegate {
private val crateGraphCachedValue: CachedValue<CrateGraph> = CachedValueImpl {
val result = buildCrateGraph(project.cargoProjects.allProjects)
CachedValueProvider.Result(result, cargoProjectsModTracker, VirtualFileManager.VFS_STRUCTURE_MODIFICATIONS)
}

private val crateGraph: CrateGraph
get() = crateGraphCachedValue.value

override val topSortedCrates: List<Crate>
get() {
checkReadAccessAllowed()
Expand All @@ -64,6 +69,8 @@ class CrateGraphServiceImpl(val project: Project) : CrateGraphService {
return rootModFile.applyWithSymlink { if (it is VirtualFileWithId) findCrateById(it.id) else null }
}

@TestOnly
fun hasUpToDateGraph(): Boolean = crateGraphCachedValue.hasUpToDateValue()
}

private data class CrateGraph(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Use of this source code is governed by the MIT license that can be
* found in the LICENSE file.
*/

package org.rustSlowTests

import com.intellij.util.ref.GCUtil
import org.rust.RsTestBase
import org.rust.lang.core.crate.crateGraph
import org.rust.lang.core.crate.impl.CrateGraphServiceImpl
import org.rust.lang.core.psi.RsStructItem
import org.rust.lang.core.psi.RsTraitItem
import org.rust.lang.core.psi.ext.descendantsOfType
import org.rust.lang.core.psi.ext.withSubst
import org.rust.lang.core.resolve.RsImplIndexAndTypeAliasCache
import org.rust.lang.core.types.TraitRef
import org.rust.lang.core.types.TyFingerprint
import org.rust.lang.core.types.implLookup

/**
* Some tests that involve [GCUtil.tryGcSoftlyReachableObjects] invocation
*/
class RsGcSoftlyReachableObjectsCacheTest : RsTestBase() {
/**
* Issue https://github.com/intellij-rust/intellij-rust/issues/9432
*
* A kind of test for [org.rust.lang.core.crate.impl.CargoBasedCrate.equals] and
* [org.rust.lang.core.crate.hasTransitiveDependencyOrSelf].
*/
fun `test trait selection works after GC collects the crate graph object`() {
InlineFile("""
struct S;
trait Trait {}
impl Trait for S {}
""".trimIndent())

val structTy = myFixture.file.descendantsOfType<RsStructItem>().single().declaredType
val trait = myFixture.file.descendantsOfType<RsTraitItem>().single()

// Retrieve impl in order to retain it in the memory and in the cache
val impls = RsImplIndexAndTypeAliasCache.getInstance(project)
.findPotentialImpls(TyFingerprint.create(structTy)!!)

fun canSelectImpl(): Boolean =
trait.implLookup.canSelect(TraitRef(structTy, trait.withSubst()))

check(canSelectImpl())
GCUtil.tryGcSoftlyReachableObjects { !(project.crateGraph as CrateGraphServiceImpl).hasUpToDateGraph() }
check(canSelectImpl())

check(impls.isNotEmpty()) // use `impls` in order to retain it in the memory until this moment
}
}

0 comments on commit 40bbade

Please sign in to comment.