ExternalVendorCode/corda
1
0
Fork 0
mirror of https://github.com/corda/corda.git synced 2025-02-20 09:26:41 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

CORDA-3755: Switched attachments map to a WeakHashMap (#6214)

Browse Source 
* Bump OS release version 4.6

* CORDA-3755: Switched attachments map to a WeakHashMap

* CORDA-3755: Added explicit strong references to map key.

* CORDA-3755: Keeping detekt happy.

* CORDA-3755: Test a gc in verify.

* CORDA-3755: Making detekt happy.

* CORDA-3755: Suppress warnings for weak reference test.

* CORDA-3755: Fixing build failure with attachments.

* CORDA-3755: Rewrite based on Ricks input - now handles attachment already existing in map!

* CORDA-3755: Refactor WeakReference behaviour into AttachmentsHolderImpl and provide alternate version of this class for core-deterministic.

* CORDA-3755: Added more tests for WeakHashMap.

* CORDA-3755: Ignore the tests using System.gc keep for local testing only

* CORDA-3755: Adding comment to explain the ignored tests.

* Make AttachmentsHolderImpl package-private inside core-deterministic, just like it is inside core.

* CORDA-3755: Update assertions following review comments.

* CORDA-3755: Removing import

* CORDA-3755: Removed unused var.

* CORDA-3755: Reverting files that somehow got changed in rebase.

Co-authored-by: nargas-ritu <ritu.gupta@r3.com>
Co-authored-by: Chris Rankin <chris.rankin@r3.com>
This commit is contained in:
Adel El-Beik 2020-05-12 09:51:12 +01:00 committed by GitHub
parent 308b31c3fe
commit 1547efb093
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 346 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core-deterministic/build.gradle
View File

@ -71,6 +71,7 @@ def patchCore = tasks.register('patchCore', Zip) {
exclude 'net/corda/core/crypto/SHA256DigestSupplier.class'
exclude 'net/corda/core/internal/*ToggleField*.class'
exclude 'net/corda/core/serialization/*SerializationFactory*.class'
exclude 'net/corda/core/serialization/internal/AttachmentsHolderImpl.class'
exclude 'net/corda/core/serialization/internal/CheckpointSerializationFactory*.class'
exclude 'net/corda/core/internal/rules/*.class'
}

23
core-deterministic/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsHolderImpl.kt Normal file
View File

@ -0,0 +1,23 @@
package net.corda.core.serialization.internal
import net.corda.core.contracts.Attachment
import java.net.URL
@Suppress("unused")
private class AttachmentsHolderImpl : AttachmentsHolder {
private val attachments = LinkedHashMap<URL, Pair<URL, Attachment>>()
override val size: Int get() = attachments.size
override fun getKey(key: URL): URL? {
return attachments[key]?.first
}
override fun get(key: URL): Attachment? {
return attachments[key]?.second
}
override fun set(key: URL, value: Attachment) {
attachments[key] = key to value
}
}

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

@ -1,19 +1,37 @@
package net.corda.coretests.transactions
import net.corda.core.contracts.AlwaysAcceptAttachmentConstraint
import net.corda.core.contracts.Attachment
import net.corda.core.contracts.CommandData
import net.corda.core.contracts.CommandWithParties
import net.corda.core.contracts.Contract
import net.corda.core.contracts.ContractAttachment
import net.corda.core.contracts.PrivacySalt
import net.corda.core.contracts.StateAndRef
import net.corda.core.contracts.TimeWindow
import net.corda.core.contracts.TransactionState
import net.corda.core.contracts.TransactionVerificationException
import net.corda.core.crypto.Crypto
import net.corda.core.crypto.SecureHash
import net.corda.core.identity.Party
import net.corda.core.internal.AbstractAttachment
import net.corda.core.internal.AttachmentTrustCalculator
import net.corda.core.internal.createLedgerTransaction
import net.corda.core.internal.declaredField
import net.corda.core.internal.hash
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.node.services.attachments.NodeAttachmentTrustCalculator
import net.corda.testing.common.internal.testNetworkParameters
import net.corda.node.services.attachments.NodeAttachmentTrustCalculator
import net.corda.testing.contracts.DummyContract
import net.corda.testing.core.ALICE_NAME
import net.corda.testing.core.BOB_NAME
import net.corda.testing.core.DUMMY_NOTARY_NAME
import net.corda.testing.core.SerializationEnvironmentRule
import net.corda.testing.core.TestIdentity
import net.corda.testing.core.internal.ContractJarTestUtils
import net.corda.testing.core.internal.ContractJarTestUtils.signContractJar
import net.corda.testing.internal.TestingNamedCacheFactory
import net.corda.testing.internal.fakeAttachment
@ -27,10 +45,14 @@ import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
import java.io.ByteArrayOutputStream
import java.io.InputStream
import java.net.URL
import java.nio.file.Path
import java.security.PublicKey
import kotlin.test.assertFailsWith
import kotlin.test.fail
@ -47,8 +69,21 @@ class AttachmentsClassLoaderTests {
it.toByteArray()
}
}
val ALICE = TestIdentity(ALICE_NAME, 70).party
val BOB = TestIdentity(BOB_NAME, 80).party
val dummyNotary = TestIdentity(DUMMY_NOTARY_NAME, 20)
val DUMMY_NOTARY get() = dummyNotary.party
val PROGRAM_ID: String = "net.corda.testing.contracts.MyDummyContract"
}
@Rule
@JvmField
val tempFolder = TemporaryFolder()
@Rule
@JvmField
val testSerialization = SerializationEnvironmentRule()
private lateinit var storage: MockAttachmentStorage
private lateinit var internalStorage: InternalMockAttachmentStorage
private lateinit var attachmentTrustCalculator: AttachmentTrustCalculator
@ -469,4 +504,93 @@ class AttachmentsClassLoaderTests {
createClassloader(trustedAttachment).use {}
}
@Test(timeout=300_000)
fun `attachment still available in verify after forced gc in verify`() {
tempFolder.root.toPath().let { path ->
val baseOutState = TransactionState(DummyContract.SingleOwnerState(0, ALICE), PROGRAM_ID, DUMMY_NOTARY, constraint = AlwaysAcceptAttachmentConstraint)
val inputs = emptyList<StateAndRef<*>>()
val outputs = listOf(baseOutState, baseOutState.copy(notary = ALICE), baseOutState.copy(notary = BOB))
val commands = emptyList<CommandWithParties<CommandData>>()
val content = createContractString(PROGRAM_ID)
val contractJarPath = ContractJarTestUtils.makeTestContractJar(path, PROGRAM_ID, content = content)
val attachments = createAttachments(contractJarPath)
val id = SecureHash.randomSHA256()
val timeWindow: TimeWindow? = null
val privacySalt = PrivacySalt()
val transaction = createLedgerTransaction(
inputs,
outputs,
commands,
attachments,
id,
null,
timeWindow,
privacySalt,
testNetworkParameters(),
emptyList(),
isAttachmentTrusted = { true }
)
transaction.verify()
}
}
private fun createContractString(contractName: String, versionSeed: Int = 0): String {
val pkgs = contractName.split(".")
val className = pkgs.last()
val packages = pkgs.subList(0, pkgs.size - 1)
val output = """package ${packages.joinToString(".")};
import net.corda.core.contracts.*;
import net.corda.core.transactions.*;
import java.net.URL;
import java.io.InputStream;
public class $className implements Contract {
private int seed = $versionSeed;
@Override
public void verify(LedgerTransaction tx) throws IllegalArgumentException {
System.gc();
InputStream str = this.getClass().getClassLoader().getResourceAsStream("importantDoc.pdf");
if (str == null) throw new IllegalStateException("Could not find importantDoc.pdf");
}
}
""".trimIndent()
System.out.println(output)
return output
}
private fun createAttachments(contractJarPath: Path) : List<Attachment> {
val attachment = object : AbstractAttachment({contractJarPath.inputStream().readBytes()}, uploader = "app") {
@Suppress("OverridingDeprecatedMember")
override val signers: List<Party> = emptyList()
override val signerKeys: List<PublicKey> = emptyList()
override val size: Int = 1234
override val id: SecureHash = SecureHash.sha256(attachmentData)
}
val contractAttachment = ContractAttachment(attachment, PROGRAM_ID)
return listOf(
object : AbstractAttachment({ISOLATED_CONTRACTS_JAR_PATH.openStream().readBytes()}, uploader = "app") {
@Suppress("OverridingDeprecatedMember")
override val signers: List<Party> = emptyList()
override val signerKeys: List<PublicKey> = emptyList()
override val size: Int = 1234
override val id: SecureHash = SecureHash.sha256(attachmentData)
},
object : AbstractAttachment({fakeAttachment("importantDoc.pdf", "I am a pdf!").inputStream().readBytes()
}, uploader = "app") {
@Suppress("OverridingDeprecatedMember")
override val signers: List<Party> = emptyList()
override val signerKeys: List<PublicKey> = emptyList()
override val size: Int = 1234
override val id: SecureHash = SecureHash.sha256(attachmentData)
},
contractAttachment)
}
}

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

@ -17,6 +17,7 @@ import net.corda.core.utilities.debug
import java.io.ByteArrayOutputStream
import java.io.IOException
import java.io.InputStream
import java.lang.ref.WeakReference
import java.net.*
import java.security.Permission
import java.util.*
@ -53,14 +54,6 @@ class AttachmentsClassLoader(attachments: List<Attachment>,
private val ignoreDirectories = listOf("org/jolokia/", "org/json/simple/")
private val ignorePackages = ignoreDirectories.map { it.replace("/", ".") }
@VisibleForTesting
private fun readAttachment(attachment: Attachment, filepath: String): ByteArray {
ByteArrayOutputStream().use {
attachment.extractFile(filepath, it)
return it.toByteArray()
}
}
/**
* Apply our custom factory either directly, if `URL.setURLStreamHandlerFactory` has not been called yet,
* or use a decorator and reflection to bypass the single-call-per-JVM restriction otherwise.
@ -359,8 +352,7 @@ object AttachmentsClassLoaderBuilder {
object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory {
internal const val attachmentScheme = "attachment"
// TODO - what happens if this grows too large?
private val loadedAttachments = mutableMapOf<String, Attachment>().toSynchronised()
private val loadedAttachments: AttachmentsHolder = AttachmentsHolderImpl()
override fun createURLStreamHandler(protocol: String): URLStreamHandler? {
return if (attachmentScheme == protocol) {
@ -368,34 +360,79 @@ object AttachmentURLStreamHandlerFactory : URLStreamHandlerFactory {
} else null
}
@Synchronized
fun toUrl(attachment: Attachment): URL {
val id = attachment.id.toString()
loadedAttachments[id] = attachment
return URL(attachmentScheme, "", -1, id, AttachmentURLStreamHandler)
val proposedURL = URL(attachmentScheme, "", -1, attachment.id.toString(), AttachmentURLStreamHandler)
val existingURL = loadedAttachments.getKey(proposedURL)
return if (existingURL == null) {
loadedAttachments[proposedURL] = attachment
proposedURL
} else {
existingURL
}
}
@VisibleForTesting
fun loadedAttachmentsSize(): Int = loadedAttachments.size
private object AttachmentURLStreamHandler : URLStreamHandler() {
override fun openConnection(url: URL): URLConnection {
if (url.protocol != attachmentScheme) throw IOException("Cannot handle protocol: ${url.protocol}")
val attachment = loadedAttachments[url.path] ?: throw IOException("Could not load url: $url .")
val attachment = loadedAttachments[url] ?: throw IOException("Could not load url: $url .")
return AttachmentURLConnection(url, attachment)
}
}
private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) {
override fun getContentLengthLong(): Long = attachment.size.toLong()
override fun getInputStream(): InputStream = attachment.open()
/**
* Define the permissions that [AttachmentsClassLoader] will need to
* use this [URL]. The attachment is stored in memory, and so we
* don't need any extra permissions here. But if we don't override
* [getPermission] then [AttachmentsClassLoader] will assign the
* default permission of ALL_PERMISSION to these classes'
* [java.security.ProtectionDomain]. This would be a security hole!
*/
override fun getPermission(): Permission? = null
override fun connect() {
connected = true
override fun equals(attachmentUrl: URL, otherURL: URL?): Boolean {
if (attachmentUrl.protocol != otherURL?.protocol) return false
if (attachmentUrl.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${attachmentUrl.protocol}")
return attachmentUrl.file == otherURL?.file
}
override fun hashCode(url: URL): Int {
if (url.protocol != attachmentScheme) throw IllegalArgumentException("Cannot handle protocol: ${url.protocol}")
return url.file.hashCode()
}
}
}
interface AttachmentsHolder {
val size: Int
fun getKey(key: URL): URL?
operator fun get(key: URL): Attachment?
operator fun set(key: URL, value: Attachment)
}
private class AttachmentsHolderImpl : AttachmentsHolder {
private val attachments = WeakHashMap<URL, Pair<WeakReference<URL>, Attachment>>().toSynchronised()
override val size: Int get() = attachments.size
override fun getKey(key: URL): URL? {
return attachments[key]?.first?.get()
}
override fun get(key: URL): Attachment? {
return attachments[key]?.second
}
override fun set(key: URL, value: Attachment) {
attachments[key] = WeakReference(key) to value
}
}
private class AttachmentURLConnection(url: URL, private val attachment: Attachment) : URLConnection(url) {
override fun getContentLengthLong(): Long = attachment.size.toLong()
override fun getInputStream(): InputStream = attachment.open()
/**
* Define the permissions that [AttachmentsClassLoader] will need to
* use this [URL]. The attachment is stored in memory, and so we
* don't need any extra permissions here. But if we don't override
* [getPermission] then [AttachmentsClassLoader] will assign the
* default permission of ALL_PERMISSION to these classes'
* [java.security.ProtectionDomain]. This would be a security hole!
*/
override fun getPermission(): Permission? = null
override fun connect() {
connected = true
}
}

127
core/src/test/kotlin/net/corda/core/internal/ClassLoadingUtilsTest.kt
View File

@ -5,13 +5,20 @@ import net.corda.core.contracts.ContractAttachment
import net.corda.core.contracts.ContractClassName
import net.corda.core.crypto.SecureHash
import net.corda.core.identity.Party
import net.corda.core.node.services.AttachmentId
import net.corda.core.serialization.internal.AttachmentURLStreamHandlerFactory
import net.corda.core.serialization.internal.AttachmentsClassLoader
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertSame
import org.junit.Ignore
import org.junit.Test
import java.io.ByteArrayOutputStream
import java.lang.ref.ReferenceQueue
import java.lang.ref.WeakReference
import java.net.URL
import java.net.URLClassLoader
import java.security.PublicKey
import java.util.jar.JarOutputStream
@ -120,9 +127,125 @@ class ClassLoadingUtilsTest {
}
}
private fun signedAttachment(data: ByteArray, vararg parties: Party) = ContractAttachment.create(
@Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.")
@Test(timeout=300_000)
@Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE")
fun `test weak reference removed from map`() {
val jarData = with(ByteArrayOutputStream()) {
val internalName = STANDALONE_CLASS_NAME.asInternalName
JarOutputStream(this, Manifest()).use {
it.setLevel(NO_COMPRESSION)
it.setMethod(DEFLATED)
it.putNextEntry(directoryEntry("com"))
it.putNextEntry(directoryEntry("com/example"))
it.putNextEntry(classEntry(internalName))
it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName))
}
toByteArray()
}
val attachment = signedAttachment(jarData)
var url: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment)
val referenceQueue: ReferenceQueue<URL> = ReferenceQueue()
val weakReference = WeakReference<URL>(url, referenceQueue)
assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
// Clear strong reference
url = null
System.gc()
val ref = referenceQueue.remove(100000)
assertSame(weakReference, ref)
assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
}
@Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.")
@Test(timeout=300_000)
@Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE")
fun `test adding same attachment twice then removing`() {
val jarData = with(ByteArrayOutputStream()) {
val internalName = STANDALONE_CLASS_NAME.asInternalName
JarOutputStream(this, Manifest()).use {
it.setLevel(NO_COMPRESSION)
it.setMethod(DEFLATED)
it.putNextEntry(directoryEntry("com"))
it.putNextEntry(directoryEntry("com/example"))
it.putNextEntry(classEntry(internalName))
it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName))
}
toByteArray()
}
val attachment1 = signedAttachment(jarData)
val attachment2 = signedAttachment(jarData)
var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1)
var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2)
val referenceQueue1: ReferenceQueue<URL> = ReferenceQueue()
val weakReference1 = WeakReference<URL>(url1, referenceQueue1)
val referenceQueue2: ReferenceQueue<URL> = ReferenceQueue()
val weakReference2 = WeakReference<URL>(url2, referenceQueue2)
assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
url1 = null
System.gc()
val ref1 = referenceQueue1.remove(500)
assertNull(ref1)
assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
url2 = null
System.gc()
val ref2 = referenceQueue2.remove(100000)
assertSame(weakReference2, ref2)
assertSame(weakReference1, referenceQueue1.poll())
assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
}
@Ignore("Using System.gc in this test which has no guarantees when/if gc occurs.")
@Test(timeout=300_000)
@Suppress("ExplicitGarbageCollectionCall", "UNUSED_VALUE")
fun `test adding two different attachments then removing`() {
val jarData1 = with(ByteArrayOutputStream()) {
val internalName = STANDALONE_CLASS_NAME.asInternalName
JarOutputStream(this, Manifest()).use {
it.setLevel(NO_COMPRESSION)
it.setMethod(DEFLATED)
it.putNextEntry(directoryEntry("com"))
it.putNextEntry(directoryEntry("com/example"))
it.putNextEntry(classEntry(internalName))
it.write(TemplateClassWithEmptyConstructor::class.java.renameTo(internalName))
}
toByteArray()
}
val attachment1 = signedAttachment(jarData1)
val attachment2 = signedAttachment(jarData1, id = SecureHash.randomSHA256())
var url1: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment1)
var url2: URL? = AttachmentURLStreamHandlerFactory.toUrl(attachment2)
val referenceQueue1: ReferenceQueue<URL> = ReferenceQueue()
val weakReference1 = WeakReference<URL>(url1, referenceQueue1)
val referenceQueue2: ReferenceQueue<URL> = ReferenceQueue()
val weakReference2 = WeakReference<URL>(url2, referenceQueue2)
assertEquals(2, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
url1 = null
System.gc()
val ref1 = referenceQueue1.remove(100000)
assertSame(weakReference1, ref1)
assertEquals(1, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
url2 = null
System.gc()
val ref2 = referenceQueue2.remove(100000)
assertSame(weakReference2, ref2)
assertEquals(0, AttachmentURLStreamHandlerFactory.loadedAttachmentsSize())
}
private fun signedAttachment(data: ByteArray, id: AttachmentId = contractAttachmentId,
vararg parties: Party) = ContractAttachment.create(
object : AbstractAttachment({ data }, "test") {
override val id: SecureHash get() = contractAttachmentId
override val id: SecureHash get() = id
override val signerKeys: List<PublicKey> get() = parties.map(Party::owningKey)
}, PROGRAM_ID, signerKeys = parties.map(Party::owningKey)

10
testing/core-test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt
View File

@ -59,12 +59,14 @@ object ContractJarTestUtils {
return workingDir.resolve(jarName) to signer
}
@Suppress("LongParameterList")
@JvmOverloads
fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0): Path {
fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0,
content: String? = null): Path {
val packages = contractName.split(".")
val jarName = "attachment-${packages.last()}-$version-$versionSeed-${(if (signed) "signed" else "")}.jar"
val className = packages.last()
createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed)
createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed, content)
workingDir.createJar(jarName, "${contractName.replace(".", "/")}.class")
workingDir.addManifest(jarName, Pair(Attributes.Name(CORDAPP_CONTRACT_VERSION), version.toString()))
return workingDir.resolve(jarName)
@ -87,8 +89,8 @@ object ContractJarTestUtils {
return workingDir.resolve(jarName)
}
private fun createTestClass(workingDir: Path, className: String, packages: List<String>, versionSeed: Int = 0): Path {
val newClass = """package ${packages.joinToString(".")};
private fun createTestClass(workingDir: Path, className: String, packages: List<String>, versionSeed: Int = 0, content: String? = null): Path {
val newClass = content ?: """package ${packages.joinToString(".")};
import net.corda.core.contracts.*;
import net.corda.core.transactions.*;