CORDA-3769: Switched attachments class loader cache to use caffeine (c…

…orda#6326)

* CORDA-3769: Switched attachments class loader cache to use caffeine with original implementation used by determinstic core.

* CORDA-3769: Removed default ctor arguments.

* CORDA-3769: Switched mapping function to Function type to avoid synthetic method being generated.

* CORDA-3769: Now using a cache created from NamedCacheFactory for the attachments class loader cache.

* CORDA-3769: Making detekt happy.

* CORDA-3769: The finality tests now check for UntrustedAttachmentsException which will actually happen in reality.

* CORDA-3769: Refactored after review comments.

* CORDA-3769: Removed the AttachmentsClassLoaderSimpleCacheImpl as DJVM does not need it. Also updated due to review comments.

* CORDA-3769: Removed the generic parameters from AttachmentsClassLoader.

* CORDA-3769: Removed unused imports.

* CORDA-3769: Updates from review comments.

* CORDA-3769: Updated following review comments. MigrationServicesForResolution now uses cache factory. Ctor updated for AttachmentsClassLoaderSimpleCacheImpl.

* CORDA-3769: Reduced max class loader cache size

* CORDA-3769: Fixed the attachments class loader cache size to a fixed default

* CORDA-3769: Switched attachments class loader size to be reduced by fixed value.

core-tests/src/test/kotlin/net/corda/coretests/flows/ReceiveFinalityFlowTest.kt

@@ -53,7 +53,7 @@ class ReceiveFinalityFlowTest {
 
         val paymentReceiverId = paymentReceiverFuture.getOrThrow()
         assertThat(bob.services.vaultService.queryBy<FungibleAsset<*>>().states).isEmpty()
-        bob.assertFlowSentForObservationDueToConstraintError(paymentReceiverId)
+        bob.assertFlowSentForObservationDueToUntrustedAttachmentsException(paymentReceiverId)
 
         // Restart Bob with the contracts CorDapp so that it can recover from the error
         bob = mockNet.restartNode(bob, parameters = InternalMockNodeParameters(additionalCordapps = listOf(FINANCE_CONTRACTS_CORDAPP)))
@@ -69,14 +69,14 @@ class ReceiveFinalityFlowTest {
                 .ofType(R::class.java)
     }
 
-    private fun TestStartedNode.assertFlowSentForObservationDueToConstraintError(runId: StateMachineRunId) {
+    private fun TestStartedNode.assertFlowSentForObservationDueToUntrustedAttachmentsException(runId: StateMachineRunId) {
         val observation = medicalRecordsOfType<Flow>()
                 .filter { it.flowId == runId }
                 .toBlocking()
                 .first()
         assertThat(observation.outcome).isEqualTo(Outcome.OVERNIGHT_OBSERVATION)
         assertThat(observation.by).contains(FinalityDoctor)
         val error = observation.errors.single()
-        assertThat(error).isInstanceOf(TransactionVerificationException.ContractConstraintRejection::class.java)
+        assertThat(error).isInstanceOf(TransactionVerificationException.UntrustedAttachmentsException::class.java)
     }
 }

core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderSerializationTests.kt

@@ -55,7 +55,7 @@ class AttachmentsClassLoaderSerializationTests {
                 arrayOf(isolatedId, att1, att2).map { storage.openAttachment(it)!! },
                 testNetworkParameters(),
                 SecureHash.zeroHash,
-                { attachmentTrustCalculator.calculate(it) }) { classLoader ->
+                { attachmentTrustCalculator.calculate(it) }, attachmentsClassLoaderCache = null) { classLoader ->
             val contractClass = Class.forName(ISOLATED_CONTRACT_CLASS_NAME, true, classLoader)
             val contract = contractClass.getDeclaredConstructor().newInstance() as Contract
             assertEquals("helloworld", contract.declaredField<Any?>("magicString").value)

core-tests/src/test/kotlin/net/corda/coretests/transactions/AttachmentsClassLoaderTests.kt

@@ -23,6 +23,7 @@ import net.corda.core.internal.inputStream
 import net.corda.core.node.NetworkParameters
 import net.corda.core.node.services.AttachmentId
 import net.corda.core.serialization.internal.AttachmentsClassLoader
+import net.corda.core.serialization.internal.AttachmentsClassLoaderCacheImpl
 import net.corda.testing.common.internal.testNetworkParameters
 import net.corda.node.services.attachments.NodeAttachmentTrustCalculator
 import net.corda.testing.contracts.DummyContract
@@ -521,6 +522,7 @@ class AttachmentsClassLoaderTests {
             val id = SecureHash.randomSHA256()
             val timeWindow: TimeWindow? = null
             val privacySalt = PrivacySalt()
+            val attachmentsClassLoaderCache = AttachmentsClassLoaderCacheImpl(cacheFactory)
             val transaction = createLedgerTransaction(
                     inputs,
                     outputs,
@@ -532,7 +534,8 @@ class AttachmentsClassLoaderTests {
                     privacySalt,
                     testNetworkParameters(),
                     emptyList(),
-                    isAttachmentTrusted = { true }
+                    isAttachmentTrusted = { true },
+                    attachmentsClassLoaderCache = attachmentsClassLoaderCache
             )
             transaction.verify()
         }

core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionTests.kt

@@ -10,6 +10,7 @@ import net.corda.core.internal.AbstractAttachment
 import net.corda.core.internal.TESTDSL_UPLOADER
 import net.corda.core.internal.createLedgerTransaction
 import net.corda.core.node.NotaryInfo
+import net.corda.core.serialization.internal.AttachmentsClassLoaderCacheImpl
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.WireTransaction
 import net.corda.testing.common.internal.testNetworkParameters
@@ -18,6 +19,7 @@ import net.corda.testing.core.*
 import net.corda.testing.internal.createWireTransaction
 import net.corda.testing.internal.fakeAttachment
 import net.corda.coretesting.internal.rigorousMock
+import net.corda.testing.internal.TestingNamedCacheFactory
 import org.junit.Rule
 import org.junit.Test
 import java.math.BigInteger
@@ -131,6 +133,7 @@ class TransactionTests {
         val id = SecureHash.randomSHA256()
         val timeWindow: TimeWindow? = null
         val privacySalt = PrivacySalt()
+        val attachmentsClassLoaderCache = AttachmentsClassLoaderCacheImpl(TestingNamedCacheFactory())
         val transaction = createLedgerTransaction(
                 inputs,
                 outputs,
@@ -142,7 +145,8 @@ class TransactionTests {
                 privacySalt,
                 testNetworkParameters(),
                 emptyList(),
-                isAttachmentTrusted = { true }
+                isAttachmentTrusted = { true },
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache
         )
 
         transaction.verify()
@@ -183,6 +187,7 @@ class TransactionTests {
         val id = SecureHash.randomSHA256()
         val timeWindow: TimeWindow? = null
         val privacySalt = PrivacySalt()
+        val attachmentsClassLoaderCache = AttachmentsClassLoaderCacheImpl(TestingNamedCacheFactory())
 
         fun buildTransaction() = createLedgerTransaction(
                 inputs,
@@ -195,7 +200,8 @@ class TransactionTests {
                 privacySalt,
                 testNetworkParameters(notaries = listOf(NotaryInfo(DUMMY_NOTARY, true))),
                 emptyList(),
-                isAttachmentTrusted = { true }
+                isAttachmentTrusted = { true },
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache
         )
 
         assertFailsWith<TransactionVerificationException.NotaryChangeInWrongTransactionType> { buildTransaction().verify() }

core/src/main/kotlin/net/corda/core/internal/ServiceHubCoreInternal.kt

@@ -5,6 +5,7 @@ import net.corda.core.DeleteForDJVM
 import net.corda.core.internal.notary.NotaryService
 import net.corda.core.node.ServiceHub
 import net.corda.core.node.StatesToRecord
+import net.corda.core.serialization.internal.AttachmentsClassLoaderCache
 import java.util.concurrent.ExecutorService
 
 // TODO: This should really be called ServiceHubInternal but that name is already taken by net.corda.node.services.api.ServiceHubInternal.
@@ -21,6 +22,8 @@ interface ServiceHubCoreInternal : ServiceHub {
     val notaryService: NotaryService?
 
     fun createTransactionsResolver(flow: ResolveTransactionsFlow): TransactionsResolver
+
+    val attachmentsClassLoaderCache: AttachmentsClassLoaderCache
 }
 
 interface TransactionsResolver {

core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt

@@ -1,5 +1,8 @@
 package net.corda.core.serialization.internal
 
+import com.github.benmanes.caffeine.cache.Cache
+import com.github.benmanes.caffeine.cache.Caffeine
+import net.corda.core.DeleteForDJVM
 import net.corda.core.contracts.Attachment
 import net.corda.core.contracts.ContractAttachment
 import net.corda.core.contracts.TransactionVerificationException
@@ -21,6 +24,7 @@ import java.lang.ref.WeakReference
 import java.net.*
 import java.security.Permission
 import java.util.*
+import java.util.function.Function
 
 /**
  * A custom ClassLoader that knows how to load classes from a set of attachments. The attachments themselves only
@@ -289,31 +293,27 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
  */
 @VisibleForTesting
 object AttachmentsClassLoaderBuilder {
-    private const val CACHE_SIZE = 1000
+    const val CACHE_SIZE = 16
 
-    // We use a set here because the ordering of attachments doesn't affect code execution, due to the no
-    // overlap rule, and attachments don't have any particular ordering enforced by the builders. So we
-    // can just do unordered comparisons here. But the same attachments run with different network parameters
-    // may behave differently, so that has to be a part of the cache key.
-    private data class Key(val hashes: Set<SecureHash>, val params: NetworkParameters)
-
-    // This runs in the DJVM so it can't use caffeine.
-    private val cache: MutableMap<Key, SerializationContext> = createSimpleCache<Key, SerializationContext>(CACHE_SIZE).toSynchronised()
+    private val fallBackCache: AttachmentsClassLoaderCache = AttachmentsClassLoaderSimpleCacheImpl(CACHE_SIZE)
 
     /**
      * Runs the given block with serialization execution context set up with a (possibly cached) attachments classloader.
      *
      * @param txId The transaction ID that triggered this request; it's unused except for error messages and exceptions that can occur during setup.
      */
+    @Suppress("LongParameterList")
     fun <T> withAttachmentsClassloaderContext(attachments: List<Attachment>,
                                               params: NetworkParameters,
                                               txId: SecureHash,
                                               isAttachmentTrusted: (Attachment) -> Boolean,
                                               parent: ClassLoader = ClassLoader.getSystemClassLoader(),
+                                              attachmentsClassLoaderCache: AttachmentsClassLoaderCache?,
                                               block: (ClassLoader) -> T): T {
         val attachmentIds = attachments.map(Attachment::id).toSet()
 
-        val serializationContext = cache.computeIfAbsent(Key(attachmentIds, params)) {
+        val cache = attachmentsClassLoaderCache ?: fallBackCache
+        val serializationContext = cache.computeIfAbsent(AttachmentsClassLoaderKey(attachmentIds, params), Function {
             // Create classloader and load serializers, whitelisted classes
             val transactionClassLoader = AttachmentsClassLoader(attachments, params, txId, isAttachmentTrusted, parent)
             val serializers = try {
@@ -336,7 +336,7 @@ object AttachmentsClassLoaderBuilder {
                     .withWhitelist(whitelistedClasses)
                     .withCustomSerializers(serializers)
                     .withoutCarpenter()
-        }
+        })
 
         // Deserialize all relevant classes in the transaction classloader.
         return SerializationFactory.defaultFactory.withCurrentContext(serializationContext) {
@@ -420,6 +420,36 @@ private class AttachmentsHolderImpl : AttachmentsHolder {
     }
 }
 
+interface AttachmentsClassLoaderCache {
+    fun computeIfAbsent(key: AttachmentsClassLoaderKey, mappingFunction: Function<in AttachmentsClassLoaderKey, out SerializationContext>): SerializationContext
+}
+
+@DeleteForDJVM
+class AttachmentsClassLoaderCacheImpl(cacheFactory: NamedCacheFactory) : SingletonSerializeAsToken(), AttachmentsClassLoaderCache {
+
+    private val cache: Cache<AttachmentsClassLoaderKey, SerializationContext> = cacheFactory.buildNamed(Caffeine.newBuilder(), "AttachmentsClassLoader_cache")
+
+    override fun computeIfAbsent(key: AttachmentsClassLoaderKey, mappingFunction: Function<in AttachmentsClassLoaderKey, out SerializationContext>): SerializationContext {
+        return cache.get(key, mappingFunction)  ?: throw NullPointerException("null returned from cache mapping function")
+    }
+}
+
+class AttachmentsClassLoaderSimpleCacheImpl(cacheSize: Int) : AttachmentsClassLoaderCache {
+
+    private val cache: MutableMap<AttachmentsClassLoaderKey, SerializationContext>
+            = createSimpleCache<AttachmentsClassLoaderKey, SerializationContext>(cacheSize).toSynchronised()
+
+    override fun computeIfAbsent(key: AttachmentsClassLoaderKey, mappingFunction: Function<in AttachmentsClassLoaderKey, out SerializationContext>): SerializationContext {
+        return cache.computeIfAbsent(key, mappingFunction)
+    }
+}
+
+// We use a set here because the ordering of attachments doesn't affect code execution, due to the no
+// overlap rule, and attachments don't have any particular ordering enforced by the builders. So we
+// can just do unordered comparisons here. But the same attachments run with different network parameters
+// may behave differently, so that has to be a part of the cache key.
+data class AttachmentsClassLoaderKey(val hashes: Set<SecureHash>, val params: NetworkParameters)
+
 private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) {
     override fun getContentLengthLong(): Long = attachment.size.toLong()
     override fun getInputStream(): InputStream = attachment.open()

core/src/main/kotlin/net/corda/core/transactions/ContractUpgradeTransactions.kt

@@ -153,7 +153,8 @@ data class ContractUpgradeWireTransaction(
                 listOf(legacyAttachment, upgradedAttachment),
                 params,
                 id,
-                { (services as ServiceHubCoreInternal).attachmentTrustCalculator.calculate(it) }) { transactionClassLoader ->
+                { (services as ServiceHubCoreInternal).attachmentTrustCalculator.calculate(it) },
+                attachmentsClassLoaderCache = (services as ServiceHubCoreInternal).attachmentsClassLoaderCache) { transactionClassLoader ->
             val resolvedInput = binaryInput.deserialize()
             val upgradedContract = upgradedContract(upgradedContractClassName, transactionClassLoader)
             val outputState = calculateUpgradedState(resolvedInput, upgradedContract, upgradedAttachment)

core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt

@@ -26,6 +26,7 @@ import net.corda.core.internal.deserialiseComponentGroup
 import net.corda.core.internal.isUploaderTrusted
 import net.corda.core.internal.uncheckedCast
 import net.corda.core.node.NetworkParameters
+import net.corda.core.serialization.internal.AttachmentsClassLoaderCache
 import net.corda.core.serialization.internal.AttachmentsClassLoaderBuilder
 import net.corda.core.utilities.contextLogger
 import java.util.Collections.unmodifiableList
@@ -87,7 +88,8 @@ private constructor(
         private val serializedInputs: List<SerializedStateAndRef>?,
         private val serializedReferences: List<SerializedStateAndRef>?,
         private val isAttachmentTrusted: (Attachment) -> Boolean,
-        private val verifierFactory: (LedgerTransaction, ClassLoader) -> Verifier
+        private val verifierFactory: (LedgerTransaction, ClassLoader) -> Verifier,
+        private val attachmentsClassLoaderCache: AttachmentsClassLoaderCache?
 ) : FullTransaction() {
 
     init {
@@ -124,7 +126,8 @@ private constructor(
                 componentGroups: List<ComponentGroup>? = null,
                 serializedInputs: List<SerializedStateAndRef>? = null,
                 serializedReferences: List<SerializedStateAndRef>? = null,
-                isAttachmentTrusted: (Attachment) -> Boolean
+                isAttachmentTrusted: (Attachment) -> Boolean,
+                attachmentsClassLoaderCache: AttachmentsClassLoaderCache?
         ): LedgerTransaction {
             return LedgerTransaction(
                 inputs = inputs,
@@ -141,7 +144,8 @@ private constructor(
                 serializedInputs = protect(serializedInputs),
                 serializedReferences = protect(serializedReferences),
                 isAttachmentTrusted = isAttachmentTrusted,
-                verifierFactory = ::BasicVerifier
+                verifierFactory = ::BasicVerifier,
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache
             )
         }
 
@@ -176,7 +180,8 @@ private constructor(
                 serializedInputs = null,
                 serializedReferences = null,
                 isAttachmentTrusted = { true },
-                verifierFactory = ::BasicVerifier
+                verifierFactory = ::BasicVerifier,
+                attachmentsClassLoaderCache = null
             )
         }
     }
@@ -218,7 +223,8 @@ private constructor(
                 txAttachments,
                 getParamsWithGoo(),
                 id,
-                isAttachmentTrusted = isAttachmentTrusted) { transactionClassLoader ->
+                isAttachmentTrusted = isAttachmentTrusted,
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache) { transactionClassLoader ->
             // Create a copy of the outer LedgerTransaction which deserializes all fields inside the [transactionClassLoader].
             // Only the copy will be used for verification, and the outer shell will be discarded.
             // This artifice is required to preserve backwards compatibility.
@@ -254,7 +260,8 @@ private constructor(
         serializedInputs = serializedInputs,
         serializedReferences = serializedReferences,
         isAttachmentTrusted = isAttachmentTrusted,
-        verifierFactory = alternateVerifier
+        verifierFactory = alternateVerifier,
+        attachmentsClassLoaderCache = attachmentsClassLoaderCache
     )
 
     // Read network parameters with backwards compatibility goo.
@@ -320,7 +327,8 @@ private constructor(
                     serializedInputs = serializedInputs,
                     serializedReferences = serializedReferences,
                     isAttachmentTrusted = isAttachmentTrusted,
-                    verifierFactory = verifierFactory
+                    verifierFactory = verifierFactory,
+                    attachmentsClassLoaderCache = attachmentsClassLoaderCache
             )
         } else {
             // This branch is only present for backwards compatibility.
@@ -704,7 +712,8 @@ private constructor(
             serializedInputs = null,
             serializedReferences = null,
             isAttachmentTrusted = { it.isUploaderTrusted() },
-            verifierFactory = ::BasicVerifier
+            verifierFactory = ::BasicVerifier,
+            attachmentsClassLoaderCache = null
     )
 
     @Deprecated("LedgerTransaction should not be created directly, use WireTransaction.toLedgerTransaction instead.")
@@ -733,7 +742,8 @@ private constructor(
             serializedInputs = null,
             serializedReferences = null,
             isAttachmentTrusted = { it.isUploaderTrusted() },
-            verifierFactory = ::BasicVerifier
+            verifierFactory = ::BasicVerifier,
+            attachmentsClassLoaderCache = null
     )
 
     @Deprecated("LedgerTransactions should not be created directly, use WireTransaction.toLedgerTransaction instead.")
@@ -761,7 +771,8 @@ private constructor(
                 serializedInputs = serializedInputs,
                 serializedReferences = serializedReferences,
                 isAttachmentTrusted = isAttachmentTrusted,
-                verifierFactory = verifierFactory
+                verifierFactory = verifierFactory,
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache
         )
     }
 
@@ -791,7 +802,8 @@ private constructor(
                 serializedInputs = serializedInputs,
                 serializedReferences = serializedReferences,
                 isAttachmentTrusted = isAttachmentTrusted,
-                verifierFactory = verifierFactory
+                verifierFactory = verifierFactory,
+                attachmentsClassLoaderCache = attachmentsClassLoaderCache
         )
     }
 }