Added background checkpoint checker to make sure they're at least deserialisable

core/src/main/kotlin/net/corda/core/serialization/CordaClassResolver.kt

@@ -1,6 +1,8 @@
 package net.corda.core.serialization
 
 import com.esotericsoftware.kryo.*
+import com.esotericsoftware.kryo.io.Input
+import com.esotericsoftware.kryo.io.Output
 import com.esotericsoftware.kryo.util.DefaultClassResolver
 import com.esotericsoftware.kryo.util.Util
 import net.corda.core.node.AttachmentsClassLoader
@@ -77,11 +79,18 @@ class CordaClassResolver(val whitelist: ClassWhitelist, val amqpEnabled: Boolean
         // case for flow checkpoints (ignoring all cases where AMQP is disabled) since our top level messaging data structures
         // are annotated and once we enter AMQP serialisation we stay with it for the entire object subgraph.
         if (!hasAnnotation || !amqpEnabled) {
+            val objectInstance = try {
+                type.kotlin.objectInstance
+            } catch (t: Throwable) {
+                // objectInstance will throw if the type is something like a lambda
+                null
+            }
             // We have to set reference to true, since the flag influences how String fields are treated and we want it to be consistent.
             val references = kryo.references
             try {
                 kryo.references = true
-                return register(Registration(type, kryo.getDefaultSerializer(type), NAME.toInt()))
+                val serializer = if (objectInstance != null) KotlinObjectSerializer(objectInstance) else kryo.getDefaultSerializer(type)
+                return register(Registration(type, serializer, NAME.toInt()))
             } finally {
                 kryo.references = references
             }
@@ -91,6 +100,12 @@ class CordaClassResolver(val whitelist: ClassWhitelist, val amqpEnabled: Boolean
         }
     }
 
+    // Trivial Serializer which simply returns the given instance which we already know is a Kotlin object
+    private class KotlinObjectSerializer(val objectInstance: Any) : Serializer<Any>() {
+        override fun read(kryo: Kryo, input: Input, type: Class<Any>): Any = objectInstance
+        override fun write(kryo: Kryo, output: Output, obj: Any) = Unit
+    }
+
     // We don't allow the annotation for classes in attachments for now.  The class will be on the main classpath if we have the CorDapp installed.
     // We also do not allow extension of KryoSerializable for annotated classes, or combination with @DefaultSerializer for custom serialisation.
     // TODO: Later we can support annotations on attachment classes and spin up a proxy via bytecode that we know is harmless.
@@ -165,8 +180,6 @@ class GlobalTransientClassWhiteList(val delegate: ClassWhitelist) : MutableClass
 /**
  * This class is not currently used, but can be installed to log a large number of missing entries from the whitelist
  * and was used to track down the initial set.
- *
- * @suppress
  */
 @Suppress("unused")
 class LoggingWhitelist(val delegate: ClassWhitelist, val global: Boolean = true) : MutableClassWhitelist {

core/src/main/kotlin/net/corda/core/serialization/DefaultKryoCustomizer.kt

@@ -25,14 +25,16 @@ import org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPrivateCrtKey
 import org.bouncycastle.jcajce.provider.asymmetric.rsa.BCRSAPublicKey
 import org.bouncycastle.pqc.jcajce.provider.sphincs.BCSphincs256PrivateKey
 import org.bouncycastle.pqc.jcajce.provider.sphincs.BCSphincs256PublicKey
+import org.objenesis.instantiator.ObjectInstantiator
+import org.objenesis.strategy.InstantiatorStrategy
 import org.objenesis.strategy.StdInstantiatorStrategy
 import org.slf4j.Logger
 import sun.security.provider.certpath.X509CertPath
 import java.io.BufferedInputStream
 import java.io.FileInputStream
 import java.io.InputStream
+import java.lang.reflect.Modifier.isPublic
 import java.security.cert.CertPath
-import java.security.cert.X509Certificate
 import java.util.*
 
 object DefaultKryoCustomizer {
@@ -51,9 +53,7 @@ object DefaultKryoCustomizer {
             // Take the safest route here and allow subclasses to have fields named the same as super classes.
             fieldSerializerConfig.cachedFieldNameStrategy = FieldSerializer.CachedFieldNameStrategy.EXTENDED
 
-            // Allow construction of objects using a JVM backdoor that skips invoking the constructors, if there is no
+            instantiatorStrategy = CustomInstantiatorStrategy()
-            // no-arg constructor available.
-            instantiatorStrategy = Kryo.DefaultInstantiatorStrategy(StdInstantiatorStrategy())
 
             register(Arrays.asList("").javaClass, ArraysAsListSerializer())
             register(SignedTransaction::class.java, ImmutableClassSerializer(SignedTransaction::class))
@@ -119,4 +119,16 @@ object DefaultKryoCustomizer {
             pluginRegistries.forEach { it.customizeSerialization(customization) }
         }
     }
+
+    private class CustomInstantiatorStrategy : InstantiatorStrategy {
+        private val fallbackStrategy = StdInstantiatorStrategy()
+        // Use this to allow construction of objects using a JVM backdoor that skips invoking the constructors, if there
+        // is no no-arg constructor available.
+        private val defaultStrategy = Kryo.DefaultInstantiatorStrategy(fallbackStrategy)
+        override fun <T> newInstantiatorOf(type: Class<T>): ObjectInstantiator<T> {
+            // However this doesn't work for non-public classes in the java. namespace
+            val strat = if (type.name.startsWith("java.") && !isPublic(type.modifiers)) fallbackStrategy else defaultStrategy
+            return strat.newInstantiatorOf(type)
+        }
+    }
 }

core/src/main/kotlin/net/corda/core/serialization/Kryo.kt

@@ -463,6 +463,7 @@ inline fun <reified T> readListOfLength(kryo: Kryo, input: Input, minLen: Int =
 }
 
 /** Marker interface for kotlin object definitions so that they are deserialized as the singleton instance. */
+// TODO This is not needed anymore
 interface DeserializeAsKotlinObjectDef
 
 /** Serializer to deserialize kotlin object definitions marked with [DeserializeAsKotlinObjectDef]. */

core/src/main/kotlin/net/corda/core/utilities/UntrustworthyData.kt

@@ -2,6 +2,7 @@ package net.corda.core.utilities
 
 import co.paralleluniverse.fibers.Suspendable
 import net.corda.core.flows.FlowException
+import java.io.Serializable
 
 /**
  * A small utility to approximate taint tracking: if a method gives you back one of these, it means the data came from
@@ -23,11 +24,8 @@ class UntrustworthyData<out T>(private val fromUntrustedWorld: T) {
     @Throws(FlowException::class)
     fun <R> unwrap(validator: Validator<T, R>) = validator.validate(fromUntrustedWorld)
 
-    @Suppress("DEPRECATION")
+    @FunctionalInterface
-    @Deprecated("This old name was confusing, use unwrap instead", replaceWith = ReplaceWith("unwrap"))
+    interface Validator<in T, out R> : Serializable {
-    inline fun <R> validate(validator: (T) -> R) = validator(data)
-
-    interface Validator<in T, out R> {
         @Suspendable
         @Throws(FlowException::class)
         fun validate(data: T): R

core/src/test/kotlin/net/corda/flows/TransactionKeyFlowTests.kt

@@ -7,6 +7,7 @@ import net.corda.testing.ALICE
 import net.corda.testing.BOB
 import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.node.MockNetwork
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
@@ -22,6 +23,11 @@ class TransactionKeyFlowTests {
         mockNet = MockNetwork(false)
     }
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     @Test
     fun `issue key`() {
         // We run this in parallel threads to help catch any race conditions that may exist.

docs/source/corda-configuration-file.rst

@@ -128,11 +128,13 @@ path to the node's base directory.
         :password: The password
         :permissions: A list of permission strings which RPC methods can use to control access
 
-    If this field is absent or an empty list then RPC is effectively locked down. Alternatively, if it contains the string ``ALL`` then the user is permitted to use *any* RPC method. This value is intended for administrator users and for developers.
+    If this field is absent or an empty list then RPC is effectively locked down. Alternatively, if it contains the string
+    ``ALL`` then the user is permitted to use *any* RPC method. This value is intended for administrator users and for developers.
 
-:devMode: This flag indicate if the node is running in development mode. On startup, if the keystore ``<workspace>/certificates/sslkeystore.jks``
+:devMode: This flag sets the node to run in development mode. On startup, if the keystore ``<workspace>/certificates/sslkeystore.jks``
     does not exist, a developer keystore will be used if ``devMode`` is true. The node will exit if ``devMode`` is false
-    and keystore does not exist.
+    and the keystore does not exist. ``devMode`` also turns on background checking of flow checkpoints to shake out any
+    bugs in the checkpointing process.
 
 :detectPublicIp: This flag toggles the auto IP detection behaviour, it is enabled by default. On startup the node will
     attempt to discover its externally visible IP address first by looking for any public addresses on its network

node/src/main/kotlin/net/corda/node/services/statemachine/StateMachineManager.kt

@@ -13,6 +13,7 @@ import com.esotericsoftware.kryo.io.Output
 import com.esotericsoftware.kryo.pool.KryoPool
 import com.google.common.collect.HashMultimap
 import com.google.common.util.concurrent.ListenableFuture
+import com.google.common.util.concurrent.MoreExecutors
 import io.requery.util.CloseableIterator
 import net.corda.core.*
 import net.corda.core.crypto.SecureHash
@@ -35,15 +36,18 @@ import net.corda.node.services.messaging.TopicSession
 import net.corda.node.utilities.*
 import org.apache.activemq.artemis.utils.ReusableLatch
 import org.jetbrains.exposed.sql.Database
+import org.slf4j.Logger
 import rx.Observable
 import rx.subjects.PublishSubject
 import java.util.*
 import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.Executors
+import java.util.concurrent.TimeUnit.SECONDS
 import javax.annotation.concurrent.ThreadSafe
 import kotlin.collections.ArrayList
 
 /**
- * A StateMachineManager is responsible for coordination and persistence of multiple [FlowStateMachine] objects.
+ * A StateMachineManager is responsible for coordination and persistence of multiple [FlowStateMachineImpl] objects.
  * Each such object represents an instantiation of a (two-party) flow that has reached a particular point.
  *
  * An implementation of this class will persist state machines to long term storage so they can survive process restarts
@@ -75,9 +79,14 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
 
     private val quasarKryoPool = KryoPool.Builder {
         val serializer = Fiber.getFiberSerializer(false) as KryoSerializer
-        DefaultKryoCustomizer.customize(serializer.kryo)
+        val classResolver = makeNoWhitelistClassResolver().apply { setKryo(serializer.kryo) }
-        serializer.kryo.addDefaultSerializer(AutoCloseable::class.java, AutoCloseableSerialisationDetector)
+        // TODO The ClassResolver can only be set in the Kryo constructor and Quasar doesn't provide us with a way of doing that
-        serializer.kryo
+        val field = Kryo::class.java.getDeclaredField("classResolver").apply { isAccessible = true }
+        serializer.kryo.apply {
+            field.set(this, classResolver)
+            DefaultKryoCustomizer.customize(this)
+            addDefaultSerializer(AutoCloseable::class.java, AutoCloseableSerialisationDetector)
+        }
     }.build()
 
     private object AutoCloseableSerialisationDetector : Serializer<AutoCloseable>() {
@@ -107,8 +116,6 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
         }
     }
 
-    val scheduler = FiberScheduler()
-
     sealed class Change {
         abstract val logic: FlowLogic<*>
 
@@ -129,14 +136,18 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
         }
     }
 
+    private val scheduler = FiberScheduler()
     private val mutex = ThreadBox(InnerState())
+    // This thread (only enabled in dev mode) deserialises checkpoints in the background to shake out bugs in checkpoint restore.
+    private val checkpointCheckerThread = if (serviceHub.configuration.devMode) Executors.newSingleThreadExecutor() else null
+
+    @Volatile private var unrestorableCheckpoints = false
 
     // True if we're shutting down, so don't resume anything.
     @Volatile private var stopping = false
     // How many Fibers are running and not suspended.  If zero and stopping is true, then we are halted.
     private val liveFibers = ReusableLatch()
 
-
     // Monitoring support.
     private val metrics = serviceHub.monitoringService.metrics
 
@@ -225,6 +236,8 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
         // Account for any expected Fibers in a test scenario.
         liveFibers.countDown(allowedUnsuspendedFiberCount)
         liveFibers.await()
+        checkpointCheckerThread?.let { MoreExecutors.shutdownAndAwaitTermination(it, 5, SECONDS) }
+        check(!unrestorableCheckpoints) { "Unrestorable checkpoints where created, please check the logs for details." }
     }
 
     /**
@@ -239,12 +252,13 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
 
     private fun restoreFibersFromCheckpoints() {
         mutex.locked {
-            checkpointStorage.forEach {
+            checkpointStorage.forEach { checkpoint ->
                 // If a flow is added before start() then don't attempt to restore it
-                if (!stateMachines.containsValue(it)) {
+                if (!stateMachines.containsValue(checkpoint)) {
-                    val fiber = deserializeFiber(it)
+                    deserializeFiber(checkpoint, logger)?.let {
-                    initFiber(fiber)
+                        initFiber(it)
-                    stateMachines[fiber] = it
+                        stateMachines[it] = checkpoint
+                    }
                 }
                 true
             }
@@ -396,13 +410,18 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
         }
     }
 
-    private fun deserializeFiber(checkpoint: Checkpoint): FlowStateMachineImpl<*> {
+    private fun deserializeFiber(checkpoint: Checkpoint, logger: Logger): FlowStateMachineImpl<*>? {
-        return quasarKryoPool.run { kryo ->
+        return try {
+            quasarKryoPool.run { kryo ->
                 // put the map of token -> tokenized into the kryo context
                 kryo.withSerializationContext(serializationContext) {
                     checkpoint.serializedFiber.deserialize(kryo)
                 }.apply { fromCheckpoint = true }
             }
+        } catch (t: Throwable) {
+            logger.error("Encountered unrestorable checkpoint!", t)
+            null
+        }
     }
 
     private fun <T> createFiber(logic: FlowLogic<T>, flowInitiator: FlowInitiator): FlowStateMachineImpl<T> {
@@ -508,6 +527,14 @@ class StateMachineManager(val serviceHub: ServiceHubInternal,
         }
         checkpointStorage.addCheckpoint(newCheckpoint)
         checkpointingMeter.mark()
+
+        checkpointCheckerThread?.execute {
+            // Immediately check that the checkpoint is valid by deserialising it. The idea is to plug any holes we have
+            // in our testing by failing any test where unrestorable checkpoints are created.
+            if (deserializeFiber(newCheckpoint, fiber.logger) == null) {
+                unrestorableCheckpoints = true
+            }
+        }
     }
 
     private fun resumeFiber(fiber: FlowStateMachineImpl<*>) {

node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt

@@ -32,6 +32,7 @@ import net.corda.testing.node.MockNetwork.MockNode
 import net.corda.testing.sequence
 import org.apache.commons.io.IOUtils
 import org.assertj.core.api.Assertions.assertThatExceptionOfType
+import org.junit.After
 import org.junit.Assert.assertArrayEquals
 import org.junit.Before
 import org.junit.Test
@@ -76,6 +77,11 @@ class CordaRPCOpsImplTest {
         }
     }
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     @Test
     fun `cash issue accepted`() {
         val quantity = 1000L

node/src/test/kotlin/net/corda/node/messaging/AttachmentTests.kt

@@ -17,6 +17,7 @@ import net.corda.node.utilities.transaction
 import net.corda.testing.node.MockNetwork
 import net.corda.testing.node.makeTestDataSourceProperties
 import org.jetbrains.exposed.sql.Database
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.io.ByteArrayInputStream
@@ -38,12 +39,15 @@ class AttachmentTests {
     @Before
     fun setUp() {
         mockNet = MockNetwork()
-
         val dataSourceProperties = makeTestDataSourceProperties()
-
         configuration = RequeryConfiguration(dataSourceProperties)
     }
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     fun fakeAttachment(): ByteArray {
         val bs = ByteArrayOutputStream()
         val js = JarOutputStream(bs)

node/src/test/kotlin/net/corda/node/messaging/InMemoryMessagingTests.kt

@@ -7,6 +7,7 @@ import net.corda.node.services.messaging.TopicStringValidator
 import net.corda.node.services.messaging.createMessage
 import net.corda.node.services.network.NetworkMapService
 import net.corda.testing.node.MockNetwork
+import org.junit.After
 import org.junit.Test
 import java.util.*
 import kotlin.test.assertEquals
@@ -16,8 +17,13 @@ import kotlin.test.assertTrue
 class InMemoryMessagingTests {
     val mockNet = MockNetwork()
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     @Test
-    fun topicStringValidation() {
+    fun `topic string validation`() {
         TopicStringValidator.check("this.is.ok")
         TopicStringValidator.check("this.is.OkAlso")
         assertFails {

node/src/test/kotlin/net/corda/node/messaging/TwoPartyTradeFlowTests.kt

@@ -3,7 +3,6 @@ package net.corda.node.messaging
 import co.paralleluniverse.fibers.Suspendable
 import net.corda.contracts.CommercialPaper
 import net.corda.contracts.asset.*
-import net.corda.testing.contracts.fillWithSomeTestCash
 import net.corda.core.*
 import net.corda.core.contracts.*
 import net.corda.core.crypto.DigitalSignature
@@ -27,7 +26,8 @@ import net.corda.core.serialization.serialize
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.transactions.WireTransaction
-import net.corda.core.utilities.*
+import net.corda.core.utilities.LogHelper
+import net.corda.core.utilities.unwrap
 import net.corda.flows.TwoPartyTradeFlow.Buyer
 import net.corda.flows.TwoPartyTradeFlow.Seller
 import net.corda.node.internal.AbstractNode
@@ -37,6 +37,7 @@ import net.corda.node.services.persistence.DBTransactionStorage
 import net.corda.node.services.persistence.checkpoints
 import net.corda.node.utilities.transaction
 import net.corda.testing.*
+import net.corda.testing.contracts.fillWithSomeTestCash
 import net.corda.testing.node.InMemoryMessagingNetwork
 import net.corda.testing.node.MockNetwork
 import org.assertj.core.api.Assertions.assertThat
@@ -76,6 +77,7 @@ class TwoPartyTradeFlowTests {
 
     @After
     fun after() {
+        mockNet.stopNodes()
         LogHelper.reset("platform.trade", "core.contract.TransactionGroup", "recordingmap")
     }
 

node/src/test/kotlin/net/corda/node/services/MockServiceHubInternal.kt

@@ -25,6 +25,7 @@ import java.time.Clock
 
 open class MockServiceHubInternal(
         override val database: Database,
+        override val configuration: NodeConfiguration,
         val customVault: VaultService? = null,
         val customVaultQuery: VaultQueryService? = null,
         val keyManagement: KeyManagementService? = null,
@@ -60,8 +61,6 @@ open class MockServiceHubInternal(
         get() = overrideClock ?: throw UnsupportedOperationException()
     override val myInfo: NodeInfo
         get() = throw UnsupportedOperationException()
-    override val configuration: NodeConfiguration
-        get() = throw UnsupportedOperationException()
     override val monitoringService: MonitoringService = MonitoringService(MetricRegistry())
     override val rpcFlows: List<Class<out FlowLogic<*>>>
         get() = throw UnsupportedOperationException()

node/src/test/kotlin/net/corda/node/services/NotaryChangeTests.kt

@@ -8,16 +8,17 @@ import net.corda.core.identity.Party
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.seconds
 import net.corda.core.transactions.WireTransaction
-import net.corda.testing.DUMMY_NOTARY
 import net.corda.flows.NotaryChangeFlow
 import net.corda.flows.StateReplacementException
 import net.corda.node.internal.AbstractNode
 import net.corda.node.services.network.NetworkMapService
 import net.corda.node.services.transactions.SimpleNotaryService
+import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.getTestPartyAndCertificate
 import net.corda.testing.node.MockNetwork
 import org.assertj.core.api.Assertions.assertThatExceptionOfType
 import org.bouncycastle.asn1.x500.X500Name
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.time.Instant
@@ -45,6 +46,11 @@ class NotaryChangeTests {
         mockNet.runNetwork() // Clear network map registration messages
     }
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     @Test
     fun `should change notary for a state with single participant`() {
         val state = issueState(clientNodeA, oldNotaryNode)

node/src/test/kotlin/net/corda/node/services/events/NodeSchedulerServiceTest.kt

@@ -21,10 +21,12 @@ import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.utilities.AffinityExecutor
 import net.corda.node.utilities.configureDatabase
 import net.corda.node.utilities.transaction
+import net.corda.testing.getTestX509Name
 import net.corda.testing.node.InMemoryMessagingNetwork
 import net.corda.testing.node.MockKeyManagementService
 import net.corda.testing.node.TestClock
 import net.corda.testing.node.makeTestDataSourceProperties
+import net.corda.testing.testNodeConfiguration
 import org.assertj.core.api.Assertions.assertThat
 import org.bouncycastle.asn1.x500.X500Name
 import org.jetbrains.exposed.sql.Database
@@ -32,6 +34,7 @@ import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.io.Closeable
+import java.nio.file.Paths
 import java.security.PublicKey
 import java.time.Clock
 import java.time.Instant
@@ -67,7 +70,6 @@ class NodeSchedulerServiceTest : SingletonSerializeAsToken() {
         val testReference: NodeSchedulerServiceTest
     }
 
-
     @Before
     fun setup() {
         countDown = CountDownLatch(1)
@@ -87,7 +89,12 @@ class NodeSchedulerServiceTest : SingletonSerializeAsToken() {
                     InMemoryMessagingNetwork.PeerHandle(0, nullIdentity),
                     AffinityExecutor.ServiceAffinityExecutor("test", 1),
                     database)
-            services = object : MockServiceHubInternal(database, overrideClock = testClock, keyManagement = kms, network = mockMessagingService), TestReference {
+            services = object : MockServiceHubInternal(
+                    database,
+                    testNodeConfiguration(Paths.get("."), getTestX509Name("Alice")),
+                    overrideClock = testClock,
+                    keyManagement = kms,
+                    network = mockMessagingService), TestReference {
                 override val vaultService: VaultService = NodeVaultService(this, dataSourceProps)
                 override val testReference = this@NodeSchedulerServiceTest
             }

node/src/test/kotlin/net/corda/node/services/persistence/DataVendingServiceTests.kt

@@ -12,14 +12,15 @@ import net.corda.core.flows.InitiatingFlow
 import net.corda.core.identity.Party
 import net.corda.core.node.services.unconsumedStates
 import net.corda.core.transactions.SignedTransaction
-import net.corda.testing.DUMMY_NOTARY
 import net.corda.flows.BroadcastTransactionFlow.NotifyTxRequest
 import net.corda.node.services.NotifyTransactionHandler
 import net.corda.node.utilities.transaction
+import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.MEGA_CORP
 import net.corda.testing.node.MockNetwork
 import net.corda.testing.node.MockNetwork.MockNode
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
@@ -35,6 +36,11 @@ class DataVendingServiceTests {
         mockNet = MockNetwork()
     }
 
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
     @Test
     fun `notify of transaction`() {
         val (vaultServiceNode, registerNode) = mockNet.createTwoNodes()

node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt

@@ -54,6 +54,7 @@ import org.junit.Before
 import org.junit.Test
 import rx.Notification
 import rx.Observable
+import java.time.Instant
 import java.util.*
 import kotlin.reflect.KClass
 import kotlin.test.assertEquals
@@ -106,11 +107,7 @@ class FlowFrameworkTests {
 
     @Test
     fun `flow can lazily use the serviceHub in its constructor`() {
-        val flow = object : FlowLogic<Unit>() {
+        val flow = LazyServiceHubAccessFlow()
-            val lazyTime by lazy { serviceHub.clock.instant() }
-            @Suspendable
-            override fun call() = Unit
-        }
         node1.services.startFlow(flow)
         assertThat(flow.lazyTime).isNotNull()
     }
@@ -754,6 +751,12 @@ class FlowFrameworkTests {
                 .toFuture()
     }
 
+    private class LazyServiceHubAccessFlow : FlowLogic<Unit>() {
+        val lazyTime: Instant by lazy { serviceHub.clock.instant() }
+        @Suspendable
+        override fun call() = Unit
+    }
+
     private class NoOpFlow(val nonTerminating: Boolean = false) : FlowLogic<Unit>() {
         @Transient var flowStarted = false
 

node/src/test/kotlin/net/corda/node/services/transactions/NotaryServiceTests.kt

@@ -10,14 +10,15 @@ import net.corda.core.getOrThrow
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.seconds
 import net.corda.core.transactions.SignedTransaction
-import net.corda.testing.DUMMY_NOTARY
 import net.corda.flows.NotaryError
 import net.corda.flows.NotaryException
 import net.corda.flows.NotaryFlow
 import net.corda.node.internal.AbstractNode
 import net.corda.node.services.network.NetworkMapService
+import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.node.MockNetwork
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.time.Instant
@@ -30,7 +31,8 @@ class NotaryServiceTests {
     lateinit var notaryNode: MockNetwork.MockNode
     lateinit var clientNode: MockNetwork.MockNode
 
-    @Before fun setup() {
+    @Before
+    fun setup() {
         mockNet = MockNetwork()
         notaryNode = mockNet.createNode(
                 legalName = DUMMY_NOTARY.name,
@@ -39,7 +41,13 @@ class NotaryServiceTests {
         mockNet.runNetwork() // Clear network map registration messages
     }
 
-    @Test fun `should sign a unique transaction with a valid time-window`() {
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
+    @Test
+    fun `should sign a unique transaction with a valid time-window`() {
         val stx = run {
             val inputState = issueState(clientNode)
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)
@@ -52,7 +60,8 @@ class NotaryServiceTests {
         signatures.forEach { it.verify(stx.id) }
     }
 
-    @Test fun `should sign a unique transaction without a time-window`() {
+    @Test
+    fun `should sign a unique transaction without a time-window`() {
         val stx = run {
             val inputState = issueState(clientNode)
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)
@@ -64,7 +73,8 @@ class NotaryServiceTests {
         signatures.forEach { it.verify(stx.id) }
     }
 
-    @Test fun `should report error for transaction with an invalid time-window`() {
+    @Test
+    fun `should report error for transaction with an invalid time-window`() {
         val stx = run {
             val inputState = issueState(clientNode)
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)
@@ -78,7 +88,8 @@ class NotaryServiceTests {
         assertThat(ex.error).isInstanceOf(NotaryError.TimeWindowInvalid::class.java)
     }
 
-    @Test fun `should sign identical transaction multiple times (signing is idempotent)`() {
+    @Test
+    fun `should sign identical transaction multiple times (signing is idempotent)`() {
         val stx = run {
             val inputState = issueState(clientNode)
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)
@@ -95,7 +106,8 @@ class NotaryServiceTests {
         assertEquals(f1.resultFuture.getOrThrow(), f2.resultFuture.getOrThrow())
     }
 
-    @Test fun `should report conflict when inputs are reused across transactions`() {
+    @Test
+    fun `should report conflict when inputs are reused across transactions`() {
         val inputState = issueState(clientNode)
         val stx = run {
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)

node/src/test/kotlin/net/corda/node/services/transactions/ValidatingNotaryServiceTests.kt

@@ -10,16 +10,17 @@ import net.corda.core.crypto.DigitalSignature
 import net.corda.core.getOrThrow
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.transactions.SignedTransaction
-import net.corda.testing.DUMMY_NOTARY
 import net.corda.flows.NotaryError
 import net.corda.flows.NotaryException
 import net.corda.flows.NotaryFlow
 import net.corda.node.internal.AbstractNode
 import net.corda.node.services.issueInvalidState
 import net.corda.node.services.network.NetworkMapService
+import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.MEGA_CORP_KEY
 import net.corda.testing.node.MockNetwork
 import org.assertj.core.api.Assertions.assertThat
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import java.util.*
@@ -31,7 +32,8 @@ class ValidatingNotaryServiceTests {
     lateinit var notaryNode: MockNetwork.MockNode
     lateinit var clientNode: MockNetwork.MockNode
 
-    @Before fun setup() {
+    @Before
+    fun setup() {
         mockNet = MockNetwork()
         notaryNode = mockNet.createNode(
                 legalName = DUMMY_NOTARY.name,
@@ -41,7 +43,13 @@ class ValidatingNotaryServiceTests {
         mockNet.runNetwork() // Clear network map registration messages
     }
 
-    @Test fun `should report error for invalid transaction dependency`() {
+    @After
+    fun cleanUp() {
+        mockNet.stopNodes()
+    }
+
+    @Test
+    fun `should report error for invalid transaction dependency`() {
         val stx = run {
             val inputState = issueInvalidState(clientNode, notaryNode.info.notaryIdentity)
             val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState)
@@ -54,7 +62,8 @@ class ValidatingNotaryServiceTests {
         assertThat(ex.error).isInstanceOf(NotaryError.SignaturesInvalid::class.java)
     }
 
-    @Test fun `should report error for missing signatures`() {
+    @Test
+    fun `should report error for missing signatures`() {
         val expectedMissingKey = MEGA_CORP_KEY.public
         val stx = run {
             val inputState = issueState(clientNode)

samples/irs-demo/src/test/kotlin/net/corda/irs/api/NodeInterestRatesTest.kt

@@ -14,16 +14,12 @@ import net.corda.core.getOrThrow
 import net.corda.core.identity.Party
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.transactions.TransactionBuilder
-import net.corda.testing.ALICE
-import net.corda.testing.DUMMY_NOTARY
 import net.corda.core.utilities.LogHelper
 import net.corda.core.utilities.ProgressTracker
 import net.corda.irs.flows.RatesFixFlow
 import net.corda.node.utilities.configureDatabase
 import net.corda.node.utilities.transaction
-import net.corda.testing.ALICE_PUBKEY
+import net.corda.testing.*
-import net.corda.testing.MEGA_CORP
-import net.corda.testing.MEGA_CORP_KEY
 import net.corda.testing.node.MockNetwork
 import net.corda.testing.node.MockServices
 import net.corda.testing.node.makeTestDataSourceProperties
@@ -232,6 +228,7 @@ class NodeInterestRatesTest {
         val fix = tx.toWireTransaction().commands.map { it.value as Fix }.first()
         assertEquals(fixOf, fix.of)
         assertEquals("0.678".bd, fix.value)
+        mockNet.stopNodes()
     }
 
     class FilteredRatesFlow(tx: TransactionBuilder,

test-utils/src/main/kotlin/net/corda/testing/node/MockNode.kt

@@ -21,7 +21,6 @@ import net.corda.core.node.services.IdentityService
 import net.corda.core.node.services.KeyManagementService
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.utilities.loggerFor
-import net.corda.flows.TransactionKeyFlow
 import net.corda.node.internal.AbstractNode
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.identity.InMemoryIdentityService
@@ -413,7 +412,6 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
     }
 
     fun stopNodes() {
-        require(nodes.isNotEmpty())
         nodes.forEach { if (it.started) it.stop() }
     }