Avoid global synchronization (#247)

jacodb-api-jvm/src/main/kotlin/org/jacodb/api/jvm/Classes.kt

@@ -40,7 +40,8 @@ interface JcClassOrInterface : JcAnnotatedSymbol, JcAccessible {
     val signature: String?
     val isAnonymous: Boolean
 
-    fun asmNode(): ClassNode
+    fun <T> withAsmNode(body: (ClassNode) -> T): T
+
     fun bytecode(): ByteArray
 
     val superClass: JcClassOrInterface?
@@ -99,7 +100,8 @@ interface JcMethod : JcSymbol, JcAnnotatedSymbol, JcAccessible, CommonMethod {
 
     val exceptions: List<TypeName>
 
-    fun asmNode(): MethodNode
+    fun <T> withAsmNode(body: (MethodNode) -> T): T
+
     override fun flowGraph(): JcGraph
 
     val rawInstList: JcInstList<JcRawInst>

jacodb-api-jvm/src/main/kotlin/org/jacodb/api/jvm/ext/JcMethods.kt

@@ -57,23 +57,24 @@ val JcMethod.humanReadableSignature: String
 @get:JvmName("hasBody")
 val JcMethod.hasBody: Boolean
     get() {
-        return !isNative && !isAbstract && asmNode().instructions.first != null
+        return !isNative && !isAbstract && withAsmNode { it.instructions.first != null }
     }
 
 
 val JcMethod.usedMethods: List<JcMethod>
     get() {
         val cp = enclosingClass.classpath
-        val methodNode = asmNode()
         val result = LinkedHashSet<JcMethod>()
-        methodNode.instructions.forEach { instruction ->
-            when (instruction) {
-                is MethodInsnNode -> {
-                    val owner = Type.getObjectType(instruction.owner).className
-                    val clazz = cp.findClassOrNull(owner)
-                    if (clazz != null) {
-                        clazz.findMethodOrNull(instruction.name, instruction.desc)?.also {
-                            result.add(it)
+        withAsmNode { methodNode ->
+            methodNode.instructions.forEach { instruction ->
+                when (instruction) {
+                    is MethodInsnNode -> {
+                        val owner = Type.getObjectType(instruction.owner).className
+                        val clazz = cp.findClassOrNull(owner)
+                        if (clazz != null) {
+                            clazz.findMethodOrNull(instruction.name, instruction.desc)?.also {
+                                result.add(it)
+                            }
                         }
                     }
                 }
@@ -94,22 +95,23 @@ class FieldUsagesResult(
 val JcMethod.usedFields: FieldUsagesResult
     get() {
         val cp = enclosingClass.classpath
-        val methodNode = asmNode()
         val reads = LinkedHashSet<JcField>()
         val writes = LinkedHashSet<JcField>()
-        methodNode.instructions.forEach { instruction ->
-            when (instruction) {
-                is FieldInsnNode -> {
-                    val owner = Type.getObjectType(instruction.owner).className
-                    val clazz = cp.findClassOrNull(owner)
-                    if (clazz != null) {
-                        val jcClass = clazz.findFieldOrNull(instruction.name)
-                        if (jcClass != null) {
-                            when (instruction.opcode) {
-                                Opcodes.GETFIELD -> reads.add(jcClass)
-                                Opcodes.GETSTATIC -> reads.add(jcClass)
-                                Opcodes.PUTFIELD -> writes.add(jcClass)
-                                Opcodes.PUTSTATIC -> writes.add(jcClass)
+        withAsmNode { methodNode ->
+            methodNode.instructions.forEach { instruction ->
+                when (instruction) {
+                    is FieldInsnNode -> {
+                        val owner = Type.getObjectType(instruction.owner).className
+                        val clazz = cp.findClassOrNull(owner)
+                        if (clazz != null) {
+                            val jcClass = clazz.findFieldOrNull(instruction.name)
+                            if (jcClass != null) {
+                                when (instruction.opcode) {
+                                    Opcodes.GETFIELD -> reads.add(jcClass)
+                                    Opcodes.GETSTATIC -> reads.add(jcClass)
+                                    Opcodes.PUTFIELD -> writes.add(jcClass)
+                                    Opcodes.PUTSTATIC -> writes.add(jcClass)
+                                }
                             }
                         }
                     }

jacodb-approximations/src/main/kotlin/org/jacodb/approximation/TransformerIntoVirtual.kt

@@ -38,6 +38,8 @@ object TransformerIntoVirtual {
 
         val exceptions = exceptions.map { it.eliminateApproximation() }
 
+        val methodNode = withAsmNode { it } // Safe since used under synchronization in JcEnrichedVirtualMethod
+
         (EnrichedVirtualMethodBuilder()
             .name(name)
             .access(access)
@@ -46,7 +48,7 @@ object TransformerIntoVirtual {
             .featuresChain(featuresChain)
             .exceptions(exceptions)
             .annotations(annotations)
-            .asmNode(asmNode())
+            .asmNode(methodNode)
             .build()
             .also { it.bind(to) }
     }

jacodb-approximations/src/main/kotlin/org/jacodb/approximation/VirtualInstances.kt

@@ -53,7 +53,9 @@ class JcEnrichedVirtualMethod(
             it.instList(this)
         }!!.instList
 
-    override fun asmNode(): MethodNode = asmNode
+    override fun <T> withAsmNode(body: (MethodNode) -> T): T = synchronized(asmNode) {
+        body(asmNode)
+    }
 
     override fun flowGraph(): JcGraph = featuresChain.call<JcMethodExtFeature, JcFlowGraphResult> {
         it.flowGraph(this)

jacodb-core/src/main/kotlin/org/jacodb/impl/bytecode/JcClassOrInterfaceImpl.kt

@@ -96,7 +96,13 @@ class JcClassOrInterfaceImpl(
         classSource.fullAsmNode
     }
 
-    override fun asmNode() = lazyAsmNode
+    override fun <T> withAsmNode(body: (ClassNode) -> T): T {
+        val asmNode = lazyAsmNode
+        return synchronized(asmNode) {
+            body(asmNode)
+        }
+    }
+
     override fun bytecode(): ByteArray = classSource.byteCode
 
     override fun <T> extensionValue(key: String): T? {

jacodb-core/src/main/kotlin/org/jacodb/impl/bytecode/JcMethodImpl.kt

@@ -75,8 +75,14 @@ class JcMethodImpl(
 
     override val description get() = methodInfo.desc
 
-    override fun asmNode(): MethodNode {
-        return enclosingClass.asmNode().methods.first { it.name == name && it.desc == methodInfo.desc }.jsrInlined
+    override fun <T> withAsmNode(body: (MethodNode) -> T): T {
+        val methodNode = enclosingClass.withAsmNode { classNode ->
+            classNode.methods.first { it.name == name && it.desc == methodInfo.desc }
+        }
+
+        return synchronized(methodNode) {
+            body(methodNode.jsrInlined)
+        }
     }
 
     override val rawInstList: JcInstList<JcRawInst>

jacodb-core/src/main/kotlin/org/jacodb/impl/cfg/MethodNodeBuilder.kt

@@ -27,7 +27,6 @@ import org.objectweb.asm.Opcodes
 import org.objectweb.asm.Opcodes.H_GETSTATIC
 import org.objectweb.asm.Type
 import org.objectweb.asm.tree.*
-import java.util.ArrayList
 
 private val PredefinedPrimitives.smallIntegers get() = setOf(Boolean, Byte, Char, Short, Int)
 
@@ -99,14 +98,14 @@ class MethodNodeBuilder(
         mn.maxLocals = localIndex
         mn.maxStack = maxStack + 1
         //At this moment, we're just copying annotations from original method without any modifications
-        with(method.asmNode()) {
-            mn.visibleAnnotations = visibleAnnotations
-            mn.visibleTypeAnnotations = visibleTypeAnnotations
-            mn.visibleParameterAnnotations = visibleParameterAnnotations
-            mn.invisibleAnnotations = invisibleAnnotations
-            mn.invisibleTypeAnnotations = invisibleTypeAnnotations
-            mn.invisibleParameterAnnotations = invisibleParameterAnnotations
-            mn.annotationDefault = annotationDefault
+        method.withAsmNode { originalMn ->
+            mn.visibleAnnotations = originalMn.visibleAnnotations
+            mn.visibleTypeAnnotations = originalMn.visibleTypeAnnotations
+            mn.visibleParameterAnnotations = originalMn.visibleParameterAnnotations
+            mn.invisibleAnnotations = originalMn.invisibleAnnotations
+            mn.invisibleTypeAnnotations = originalMn.invisibleTypeAnnotations
+            mn.invisibleParameterAnnotations = originalMn.invisibleParameterAnnotations
+            mn.annotationDefault = originalMn.annotationDefault
 
             //            this two line of code relies on labels in method body properly organized.
 
@@ -125,7 +124,7 @@ class MethodNodeBuilder(
             staticInc = 1
         }
 
-        val variables = method.asmNode().localVariables.orEmpty().sortedBy(LocalVariableNode::index)
+        val variables = method.withAsmNode { it.localVariables.orEmpty().sortedBy(LocalVariableNode::index) }
 
         fun getName(parameter: JcParameter): String? {
             val idx = parameter.index + staticInc

jacodb-core/src/main/kotlin/org/jacodb/impl/features/classpaths/MethodInstructionsFeature.kt

@@ -39,7 +39,6 @@ class MethodInstructionsFeature(
     private val JcMethod.methodFeatures
         get() = enclosingClass.classpath.features?.filterIsInstance<JcInstExtFeature>().orEmpty()
 
-    @Synchronized
     override fun flowGraph(method: JcMethod): JcMethodExtFeature.JcFlowGraphResult {
         return JcFlowGraphResultImpl(method, JcGraphImpl(method, method.instList.instructions))
     }
@@ -52,7 +51,9 @@ class MethodInstructionsFeature(
     }
 
     override fun rawInstList(method: JcMethod): JcMethodExtFeature.JcRawInstListResult {
-        val list: JcInstList<JcRawInst> = RawInstListBuilder(method, method.asmNode(), keepLocalVariableNames).build()
+        val list: JcInstList<JcRawInst> = method.withAsmNode { methodNode ->
+            RawInstListBuilder(method, methodNode, keepLocalVariableNames).build()
+        }
         return JcRawInstListResultImpl(method, method.methodFeatures.fold(list) { value, feature ->
             feature.transformRawInstList(method, value)
         })

jacodb-core/src/main/kotlin/org/jacodb/impl/features/classpaths/virtual/JcVirtualClass.kt

@@ -80,7 +80,7 @@ open class JcVirtualClassImpl(
 
     override val simpleName: String get() = name.substringAfterLast(".")
 
-    override fun asmNode(): ClassNode {
+    override fun <T> withAsmNode(body: (ClassNode) -> T): T {
         throw IllegalStateException("Can't get ASM node for Virtual class")
     }
 

jacodb-core/src/main/kotlin/org/jacodb/impl/features/classpaths/virtual/JcVirtualMethod.kt

@@ -36,7 +36,7 @@ interface JcVirtualMethod : JcMethod {
 
     fun bind(clazz: JcClassOrInterface)
 
-    override fun asmNode() = MethodNode()
+    override fun <T> withAsmNode(body: (MethodNode) -> T): T = body(MethodNode())
 
     override val rawInstList: JcInstList<JcRawInst>
         get() = JcInstListImpl(emptyList())

jacodb-core/src/test/kotlin/org/jacodb/testing/DatabaseLifecycleTest.kt

@@ -76,7 +76,9 @@ class DatabaseLifecycleTest {
         val barKt = cp.findClass<BarKt>()
         db.awaitBackgroundJobs()
         assertTrue(testDirClone.deleteRecursively())
-        assertNotNull(barKt.declaredMethods.first().asmNode())
+        barKt.declaredMethods.first().withAsmNode { methodNode ->
+            assertNotNull(methodNode)
+        }
 
         db.refresh()
 
@@ -90,7 +92,9 @@ class DatabaseLifecycleTest {
         }
 
         with(cp.findClass<BarKt>()) {
-            assertNotNull(declaredMethods.first().asmNode())
+            declaredMethods.first().withAsmNode { methodNode ->
+                assertNotNull(methodNode)
+            }
         }
 
         cp.close()
@@ -108,7 +112,9 @@ class DatabaseLifecycleTest {
 
         assertNotNull(
             runBlocking {
-                barKt.declaredMethods.first().asmNode()
+                barKt.declaredMethods.first().withAsmNode { methodNode ->
+                    assertNotNull(methodNode)
+                }
             }
         )
     }
@@ -134,7 +140,9 @@ class DatabaseLifecycleTest {
         db.awaitBackgroundJobs() // is required for deleting jar
 
         assertTrue(guavaLibClone.deleteWithRetries(3))
-        assertNotNull(abstractCacheClass.declaredMethods.first().asmNode())
+        abstractCacheClass.declaredMethods.first().withAsmNode { methodNode ->
+            assertNotNull(methodNode)
+        }
 
         db.refresh()
         withRegistry {
@@ -156,9 +164,9 @@ class DatabaseLifecycleTest {
         val abstractCacheClass = cp.findClassOrNull<AbstractCache<*, *>>()
         assertNotNull(abstractCacheClass!!)
 
-        assertNotNull(
-            abstractCacheClass.declaredMethods.first().asmNode()
-        )
+        abstractCacheClass.declaredMethods.first().withAsmNode { methodNode ->
+            assertNotNull(methodNode)
+        }
     }
 
     @Test
@@ -170,9 +178,9 @@ class DatabaseLifecycleTest {
 
             val abstractCacheClass = findClass<AbstractCache<*, *>>()
 
-            assertNotNull(
-                abstractCacheClass.declaredMethods.first().asmNode()
-            )
+            abstractCacheClass.declaredMethods.first().withAsmNode { methodNode ->
+                assertNotNull(methodNode)
+            }
         }
         withContext(Dispatchers.IO) {
             cps.map {
@@ -193,7 +201,9 @@ class DatabaseLifecycleTest {
     fun `jar should not be blocked after method read`() = runBlocking {
         val cp = db.classpath(listOf(guavaLibClone))
         val clazz = cp.findClass<Iterators>()
-        assertNotNull(clazz.declaredMethods.first().asmNode())
+        clazz.declaredMethods.first().withAsmNode { methodNode ->
+            assertNotNull(methodNode)
+        }
         db.awaitBackgroundJobs()
         assertTrue(guavaLibClone.deleteWithRetries(3))
     }

jacodb-core/src/test/kotlin/org/jacodb/testing/ParameterNamesTest.kt

@@ -45,8 +45,9 @@ abstract class ParameterNamesTest : BaseTest() {
 
     private val JcMethod.parameterNames: List<String?>
         get() {
-            return enclosingClass.asmNode()
-                .asClassInfo(enclosingClass.bytecode()).methods.find { info -> info.name == name && info.desc == description }
+            return enclosingClass
+                .withAsmNode { it.asClassInfo(enclosingClass.bytecode()) }
+                .methods.find { info -> info.name == name && info.desc == description }
                 ?.parametersInfo?.map(ParameterInfo::name)
                 ?: parameters.map(JcParameter::name)
         }

jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/BaseInstructionsTest.kt

@@ -74,15 +74,15 @@ abstract class BaseInstructionsTest : BaseTest() {
         muteGraphChecker: Boolean = false,
     ): Class<*>? {
         try {
-            val classNode = klass.asmNode()
+            val classNode = klass.withAsmNode { it } // fixme: safe only in single-thread environment
             classNode.methods = klass.declaredMethods
                 .filter { it.enclosingClass == klass }
                 .map { method ->
                     if (method.isAbstract ||
                         method.name.contains("$\$forInline")||
                         method.name.contains("lambda$") ||
                         method.name.contains("stringConcat$")) {
-                        method.asmNode()
+                        method.withAsmNode { it } // fixme: safe only in single-thread environment
                     } else {
                         try {
                             val instructionList = method.rawInstList

jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/InstructionsTest.kt

@@ -349,7 +349,7 @@ class InstructionsTest : BaseInstructionsTest() {
 fun JcMethod.dumpInstructions(): String {
     return buildString {
         val textifier = Textifier()
-        asmNode().accept(TraceMethodVisitor(textifier))
+        withAsmNode { it.accept(TraceMethodVisitor(textifier)) }
         textifier.text.printList(this)
     }
 }

jacodb-core/src/test/kotlin/org/jacodb/testing/types/IgnoreSubstitutionProblemsTest.kt

@@ -63,8 +63,7 @@ open class IgnoreSubstitutionProblemsTest : BaseTest() {
         return runBlocking { db.classpath(listOf(target.toFile()), listOf(IgnoreSubstitutionProblems)).findClass("GenericsApiConsumer") }
     }
 
-    private fun JcClassOrInterface.tweakClass(action: ClassNode.() -> Unit = {}) {
-        val classNode = asmNode()
+    private fun JcClassOrInterface.tweakClass(action: ClassNode.() -> Unit = {}): Unit = withAsmNode { classNode ->
         classNode.action()
         val cw = JcDatabaseClassWriter(cp, ClassWriter.COMPUTE_FRAMES)
         val checker = CheckClassAdapter(cw)

jacodb-core/src/testFixtures/kotlin/org/jacodb/testing/tests/DatabaseEnvTest.kt

@@ -346,8 +346,11 @@ abstract class DatabaseEnvTest {
 
         val method = runnable.declaredMethods.first()
         assertFalse(method.hasBody)
-        assertNotNull(method.asmNode())
-        assertTrue(method.asmNode().instructions.toList().isEmpty())
+
+        method.withAsmNode { methodNode ->
+            assertNotNull(methodNode)
+            assertTrue(methodNode.instructions.toList().isEmpty())
+        }
     }
 
     @Test

jacodb-examples/src/main/java/org/jacodb/examples/JavaReadMeExamples.java

@@ -68,7 +68,7 @@ public static MethodNode findNormalDistribution() throws Exception {
         System.out.println(JcClasses.getConstructors(jcClass).size());
 
         // At this point the database read the method bytecode and return the result.
-        return jcClass.getDeclaredMethods().get(0).asmNode();
+        return jcClass.getDeclaredMethods().get(0).withAsmNode(it -> it);
     }
 
     public static void watchFileChanges() throws Exception {

jacodb-examples/src/main/kotlin/org/jacodb/examples/KotlinReadMeExamples.kt

@@ -54,7 +54,7 @@ suspend fun findNormalDistribution(): Any {
     println(jcClass.annotations.size)
 
     // At this point the database read the method bytecode and return the result.
-    return jcClass.methods[0].asmNode()
+    return jcClass.methods[0].withAsmNode { it }
 }
 
 suspend fun watchFileChanges() {