[LL] CleanableSoftValueCache: Add non-concurrent tests for value cleanup

- The non-concurrent `CleanableSoftValueCacheTest` ensures that deterministic cleanup behaves correctly. A Lincheck test can only discover differences between the single-threaded and concurrent executions of a test scenario, so it cannot find correctness issues with deterministic cleanup on its own. ^KT-61222
2024-01-17 17:26:23 +01:00
parent 53e545aa3f
commit 5207940f0b
4 changed files with 282 additions and 12 deletions
@@ -5,8 +5,10 @@

 package org.jetbrains.kotlin.analysis.test.framework.utils

+import com.intellij.mock.MockApplication
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.util.Computable
+import com.intellij.openapi.util.Disposer
 import com.intellij.psi.PsiElement
 import com.intellij.psi.PsiReference
 import com.intellij.psi.impl.source.resolve.reference.impl.PsiMultiReference
@@ -16,6 +18,7 @@ import org.jetbrains.kotlin.idea.references.KtReference
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.test.directives.model.Directive
 import org.jetbrains.kotlin.test.directives.model.RegisteredDirectives
+import org.jetbrains.kotlin.test.testFramework.resetApplicationToNull

 inline fun <T> runReadAction(crossinline runnable: () -> T): T {
    return ApplicationManager.getApplication().runReadAction(Computable { runnable() })
@@ -24,6 +27,30 @@ inline fun <T> runReadAction(crossinline runnable: () -> T): T {
 fun <R> executeOnPooledThreadInReadAction(action: () -> R): R =
    ApplicationManager.getApplication().executeOnPooledThread<R> { runReadAction(action) }.get()

+/**
+ * Executes [action] with a dummy application available via [ApplicationManager.getApplication]. This function should **only** be used if
+ * an application is needed to avoid null pointer exceptions from [ApplicationManager.getApplication] when used in simple situations. Do not
+ * use this function if you need a properly set up application and project.
+ */
+fun <R> withDummyApplication(action: () -> R): R {
+    val previousApplication = ApplicationManager.getApplication()
+    val disposable = Disposer.newDisposable("Application disposable for dummy application from Analysis API test framework")
+    try {
+        MockApplication.setUp(disposable)
+        return action()
+    } finally {
+        Disposer.dispose(disposable)
+
+        // If there is a previous application, disposal of `disposable` will reset the application manager to that application, so we won't
+        // need to nor should we reset its application to `null`. This is handled by the application argument in `resetApplicationToNull`.
+        resetApplicationToNull(previousApplication)
+
+        require(ApplicationManager.getApplication() === previousApplication) {
+            "The managed application should have been reset to the previous application or `null`."
+        }
+    }
+}
+
 fun PsiElement?.position(): String {
    if (this == null) return "(unknown)"
    return offsetToLineAndColumn(containingFile.viewProvider.document, textRange.startOffset).toString()
@@ -9,7 +9,10 @@ dependencies {

    testImplementation(project(":analysis:analysis-api"))
    testImplementation(project(":analysis:low-level-api-fir"))
+    testImplementation(projectTests(":analysis:analysis-test-framework"))
    testImplementation("org.jetbrains.kotlinx:lincheck:2.23")
+
+    testRuntimeOnly(commonDependency("org.jetbrains.intellij.deps.fastutil:intellij-deps-fastutil"))
 }

 sourceSets {
@@ -0,0 +1,246 @@
+/*
+ * Copyright 2010-2024 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.analysis.low.level.api.fir.caches
+
+import org.jetbrains.kotlin.analysis.test.framework.utils.withDummyApplication
+import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertNotNull
+import org.junit.jupiter.api.Assertions.assertNull
+import org.junit.jupiter.api.Assertions.assertSame
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+
+class CleanableSoftValueCacheTest {
+    @Test
+    fun removeCleansUpValues() {
+        val value1 = ValueWithCleanup("v1")
+        val value2 = ValueWithCleanup("v2")
+
+        val cache = setUpCache(value1, value2)
+
+        val removedValue1 = cache.remove("v1")
+        assertNull(cache.get("v1"))
+        assertSame(value1, removedValue1)
+        assertTrue(value1.isCleanedUp)
+
+        assertSame(value2, cache.get("v2"))
+        assertFalse(value2.isCleanedUp)
+
+        val removedValue2 = cache.remove("v2")
+        assertNull(cache.get("v2"))
+        assertSame(value2, removedValue2)
+        assertTrue(value2.isCleanedUp)
+    }
+
+    @Test
+    fun putCleansUpValues() {
+        val value1 = ValueWithCleanup("v1")
+        val value2 = ValueWithCleanup("v2")
+
+        val cache = setUpCache(value1, value2)
+
+        val valueReplacement1 = ValueWithCleanup("vr1")
+        val valueReplacement2 = ValueWithCleanup("vr2")
+
+        val oldValue1 = cache.put("v1", valueReplacement1)
+        assertSame(valueReplacement1, cache.get("v1"))
+        assertSame(value1, oldValue1)
+        assertTrue(value1.isCleanedUp)
+        assertFalse(valueReplacement1.isCleanedUp)
+
+        assertSame(value2, cache.get("v2"))
+        assertFalse(value2.isCleanedUp)
+
+        val oldValue2 = cache.put("v2", valueReplacement2)
+        assertSame(valueReplacement2, cache.get("v2"))
+        assertSame(value2, oldValue2)
+        assertTrue(value2.isCleanedUp)
+        assertFalse(valueReplacement2.isCleanedUp)
+    }
+
+    @Test
+    fun putAvoidsSameValueCleanup() {
+        val value1 = ValueWithCleanup("v1")
+
+        val cache = setUpCache(value1)
+
+        val oldValue = cache.put("v1", value1)
+        assertSame(value1, cache.get("v1"))
+        assertSame(value1, oldValue)
+        assertFalse(value1.isCleanedUp)
+    }
+
+    @Test
+    fun computeAddsValues() {
+        val value1 = ValueWithCleanup("v1")
+
+        val cache = setUpCache(value1)
+
+        val value2 = ValueWithCleanup("v2")
+        val newValue = cache.compute("v2") { _, oldValue ->
+            assertNull(oldValue)
+            value2
+        }
+        assertSame(value2, cache.get("v2"))
+        assertSame(value2, newValue)
+
+        assertFalse(value1.isCleanedUp)
+        assertFalse(value2.isCleanedUp)
+    }
+
+    @Test
+    fun computeCleansUpValues() {
+        val value1 = ValueWithCleanup("v1")
+        val value2 = ValueWithCleanup("v2")
+
+        val cache = setUpCache(value1, value2)
+
+        val valueReplacement1 = ValueWithCleanup("vr1")
+        val valueReplacement2: ValueWithCleanup? = null
+
+        val newValue1 = cache.compute("v1") { _, oldValue ->
+            assertSame(value1, oldValue)
+            valueReplacement1
+        }
+        assertSame(valueReplacement1, cache.get("v1"))
+        assertSame(valueReplacement1, newValue1)
+        assertTrue(value1.isCleanedUp)
+        assertFalse(valueReplacement1.isCleanedUp)
+
+        assertSame(value2, cache.get("v2"))
+        assertFalse(value2.isCleanedUp)
+
+        val newValue2 = cache.compute("v2") { _, oldValue ->
+            assertSame(value2, oldValue)
+            valueReplacement2
+        }
+        assertSame(valueReplacement2, cache.get("v2"))
+        assertSame(valueReplacement2, newValue2)
+        assertTrue(value2.isCleanedUp)
+    }
+
+    @Test
+    fun computeAvoidsSameValueCleanup() {
+        val value1 = ValueWithCleanup("v1")
+
+        val cache = setUpCache(value1)
+
+        val newValue = cache.compute("v1") { _, oldValue ->
+            assertSame(value1, oldValue)
+            value1
+        }
+        assertSame(value1, cache.get("v1"))
+        assertSame(value1, newValue)
+        assertFalse(value1.isCleanedUp)
+    }
+
+    @Test
+    fun computeIfAbsentAddsValues() {
+        val value1 = ValueWithCleanup("v1")
+
+        val cache = setUpCache(value1)
+
+        val value2 = ValueWithCleanup("v2")
+        val newValue = cache.computeIfAbsent("v2") { value2 }
+        assertSame(value2, cache.get("v2"))
+        assertSame(value2, newValue)
+
+        assertFalse(value1.isCleanedUp)
+        assertFalse(value2.isCleanedUp)
+    }
+
+    @Test
+    fun computeIfAbsentKeepsExistingValues() {
+        val value1 = ValueWithCleanup("v1")
+        val value2 = ValueWithCleanup("v2")
+
+        val cache = setUpCache(value1, value2)
+
+        val valueReplacement1 = ValueWithCleanup("vr1")
+        val valueReplacement2 = ValueWithCleanup("vr2")
+
+        val currentValue1 = cache.computeIfAbsent("v1") { valueReplacement1 }
+        assertSame(value1, cache.get("v1"))
+        assertSame(value1, currentValue1)
+        assertFalse(value1.isCleanedUp)
+        assertFalse(valueReplacement1.isCleanedUp)
+
+        assertSame(value2, cache.get("v2"))
+        assertFalse(value2.isCleanedUp)
+
+        val currentValue2 = cache.computeIfAbsent("v2") { valueReplacement2 }
+        assertSame(value2, cache.get("v2"))
+        assertSame(value2, currentValue2)
+        assertFalse(value2.isCleanedUp)
+        assertFalse(valueReplacement2.isCleanedUp)
+    }
+
+    @Test
+    fun clearCleansUpValues() {
+        val value1 = ValueWithCleanup("v1")
+        val value2 = ValueWithCleanup("v2")
+        val value3 = ValueWithCleanup("v3")
+
+        val cache = setUpCache(value1, value2, value3)
+
+        // We need an application so that `clear` can assert write access.
+        withDummyApplication {
+            cache.clear()
+        }
+
+        assertTrue(cache.isEmpty())
+        assertTrue(value1.isCleanedUp)
+        assertTrue(value2.isCleanedUp)
+        assertTrue(value3.isCleanedUp)
+    }
+
+    class ValueWithCleanup(val name: String) {
+        val cleanupMarker: CleanupMarker = CleanupMarker()
+
+        val isCleanedUp: Boolean get() = cleanupMarker.isCleanedUp
+
+        // This equality implementation is needed as we want to check that `compute` doesn't clean up a replaced value that is referentially
+        // equal to the new value, but does clean up a replaced value that is only equal to the new value by `equals`, not reference.
+        override fun equals(other: Any?): Boolean = (other as? ValueWithCleanup)?.name == name
+
+        override fun hashCode(): Int = name.hashCode()
+
+        override fun toString(): String = "ValueWithCleanup:$name"
+    }
+
+    /**
+     * [ValueWithCleanup] shouldn't be referenced from its [SoftValueCleaner], because this would make the value strongly reachable from the
+     * reference held by [CleanableSoftValueCache]. Instead, we need to keep [isCleanedUp] in this separate class.
+     *
+     * We cannot check the cleanup count in this test because `CleanableSoftValueCache` does not guarantee any specific number of cleanup
+     * calls.
+     */
+    class CleanupMarker : SoftValueCleaner<ValueWithCleanup> {
+        var isCleanedUp: Boolean = false
+
+        override fun cleanUp(value: ValueWithCleanup?) {
+            isCleanedUp = true
+        }
+    }
+
+    private fun createCache(): CleanableSoftValueCache<String, ValueWithCleanup> = CleanableSoftValueCache { it.cleanupMarker }
+
+    private fun setUpCache(vararg values: ValueWithCleanup): CleanableSoftValueCache<String, ValueWithCleanup> {
+        val cache = createCache()
+
+        values.forEach { value ->
+            cache.put(value.name, value)
+        }
+
+        cache.keys.forEach { key ->
+            val value = cache.get(key)
+            assertNotNull(value)
+            assertFalse(value!!.isCleanedUp)
+        }
+
+        return cache
+    }
+}
@@ -3896,12 +3896,6 @@
            <sha256 value="cde3341ba18a2ba262b0b7cf6c55b20c90e8d434e42c9a13e6a3f770db965a88" origin="Generated by Gradle"/>
         </artifact>
      </component>
-      <component group="org.jetbrains.kotlin" name="kotlin-stdlib-jdk7" version="1.6.21">
-         <artifact name="kotlin-stdlib-jdk7-1.6.21.jar">
-            <md5 value="73d0db6af7445088858d51a1a6843b75" origin="Generated by Gradle"/>
-            <sha256 value="f1b0634dbb94172038463020bb2dd45ca26849f8ce29d625acb0f1569d11dbee" origin="Generated by Gradle"/>
-         </artifact>
-      </component>
      <component group="org.jetbrains.kotlin" name="kotlin-stdlib-jdk7" version="1.7.10">
         <artifact name="kotlin-stdlib-jdk7-1.7.10.jar">
            <md5 value="16e9288ff1d39c8f983507e3cd72e8a8" origin="Generated by Gradle"/>
@@ -3944,12 +3938,6 @@
            <sha256 value="ac6361bf9ad1ed382c2103d9712c47cdec166232b4903ed596e8876b0681c9b7" origin="Generated by Gradle"/>
         </artifact>
      </component>
-      <component group="org.jetbrains.kotlin" name="kotlin-stdlib-jdk8" version="1.6.21">
-         <artifact name="kotlin-stdlib-jdk8-1.6.21.jar">
-            <md5 value="9e7ee18a1a5dd5bf070c7e6f706ccc9c" origin="Generated by Gradle"/>
-            <sha256 value="dab45489b47736d59fce44b80676f1947a9b6bcab10fd60e878a83bd82a6954c" origin="Generated by Gradle"/>
-         </artifact>
-      </component>
      <component group="org.jetbrains.kotlin" name="kotlin-stdlib-jdk8" version="1.7.10">
         <artifact name="kotlin-stdlib-jdk8-1.7.10.jar">
            <md5 value="83947a5800c9fae4d5e0aa15ed5f186f" origin="Generated by Gradle"/>
@@ -4212,6 +4200,12 @@
            <sha256 value="518b349498ee371f840f7ee240b942b921960c507dc9dbd853a7e83197043def" origin="Generated by Gradle"/>
         </artifact>
      </component>
+      <component group="org.jetbrains.kotlinx" name="kotlinx-coroutines-jdk8" version="1.6.4">
+         <artifact name="kotlinx-coroutines-jdk8-1.6.4.jar">
+            <md5 value="9532e16578f95c1f9bb3d199fa1c1039" origin="Generated by Gradle"/>
+            <sha256 value="88c64b8eea3eb90597d2fb0fd30f3cf782fbcdad06312e5665a618f070f02119" origin="Generated by Gradle"/>
+         </artifact>
+      </component>
      <component group="org.jetbrains.kotlinx" name="kotlinx-coroutines-jdk8" version="1.7.1">
         <artifact name="kotlinx-coroutines-jdk8-1.7.1.jar">
            <md5 value="fa342b8acc44aca4c5ba2a7ce06e58a4" origin="Generated by Gradle"/>