[CORDA-2890] - Close security manager after broker is shut down (#5425)

client/rpc/src/integration-test/kotlin/net/corda/client/rpc/CordaRPCClientTest.kt

@@ -27,6 +27,7 @@ import net.corda.testing.core.*
 import net.corda.testing.node.User
 import net.corda.testing.node.internal.NodeBasedTest
 import net.corda.testing.node.internal.ProcessUtilities
+import net.corda.testing.node.internal.poll
 import org.apache.activemq.artemis.api.core.ActiveMQSecurityException
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatExceptionOfType
@@ -37,10 +38,7 @@ import rx.subjects.PublishSubject
 import java.net.URLClassLoader
 import java.nio.file.Paths
 import java.util.*
-import java.util.concurrent.CountDownLatch
-import java.util.concurrent.Executors
-import java.util.concurrent.ScheduledExecutorService
-import java.util.concurrent.TimeUnit
+import java.util.concurrent.*
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
@@ -101,7 +99,7 @@ class CordaRPCClientTest : NodeBasedTest(listOf("net.corda.finance"), notaries =
         var successful = false
         val maxCount = 120
         var count = 0
-        CloseableExecutor(Executors.newSingleThreadScheduledExecutor()).use { scheduler ->
+        CloseableExecutor(Executors.newScheduledThreadPool(2)).use { scheduler ->
 
             val task = scheduler.scheduleAtFixedRate({
                 try {
@@ -116,8 +114,6 @@ class CordaRPCClientTest : NodeBasedTest(listOf("net.corda.finance"), notaries =
                 } catch (e: RPCException) {
                     log.info("... node is not running.")
                     nodeIsShut.onCompleted()
-                } catch (e: ActiveMQSecurityException) {
-                    // nothing here - this happens if trying to connect before the node is started
                 } catch (e: Exception) {
                     nodeIsShut.onError(e)
                 }
@@ -126,13 +122,19 @@ class CordaRPCClientTest : NodeBasedTest(listOf("net.corda.finance"), notaries =
             nodeIsShut.doOnError { error ->
                 log.error("FAILED TO SHUT DOWN NODE DUE TO", error)
                 successful = false
-                task.cancel(true)
+                task.cancel(false)
                 latch.countDown()
             }.doOnCompleted {
-                        successful = (node.node.started == null)
-                        task.cancel(true)
-                        latch.countDown()
-                    }.subscribe()
+                val nodeTerminated = try {
+                    poll(scheduler, pollName = "node's started state", check = { if (node.node.started == null) true else null })
+                            .get(10, TimeUnit.SECONDS)
+                } catch (e: Exception) {
+                    false
+                }
+                successful = nodeTerminated
+                task.cancel(false)
+                latch.countDown()
+            }.subscribe()
 
             client.start(rpcUser.username, rpcUser.password).use { rpc -> rpc.proxy.shutdown() }
 

client/rpc/src/main/kotlin/net/corda/client/rpc/internal/ReconnectingCordaRPCOps.kt

@@ -71,9 +71,6 @@ class ReconnectingCordaRPCOps private constructor(
             observersPool ?: Executors.newCachedThreadPool(),
             observersPool != null)
     private companion object {
-        // See https://r3-cev.atlassian.net/browse/CORDA-2890.
-        // TODO Once the bug is fixed, this retry logic should be removed.
-        const val MAX_RETRY_ATTEMPTS_ON_AUTH_ERROR = 3
         private val log = contextLogger()
         private fun proxy(reconnectingRPCConnection: ReconnectingRPCConnection, observersPool: ExecutorService): InternalCordaRPCOps {
             return Proxy.newProxyInstance(
@@ -159,8 +156,7 @@ class ReconnectingCordaRPCOps private constructor(
             return currentRPCConnection!!
         }
 
-        private tailrec fun establishConnectionWithRetry(retryInterval: Duration = 1.seconds, currentAuthenticationRetries: Int = 0, roundRobinIndex: Int = 0): CordaRPCConnection {
-            var _currentAuthenticationRetries = currentAuthenticationRetries
+        private tailrec fun establishConnectionWithRetry(retryInterval: Duration = 1.seconds, roundRobinIndex: Int = 0): CordaRPCConnection {
             val attemptedAddress = nodeHostAndPorts[roundRobinIndex]
             log.info("Connecting to: $attemptedAddress")
             try {
@@ -176,12 +172,8 @@ class ReconnectingCordaRPCOps private constructor(
             } catch (ex: Exception) {
                 when (ex) {
                     is ActiveMQSecurityException -> {
-                        // Happens when incorrect credentials provided.
-                        // It can happen at startup as well when the credentials are correct.
-                        if (_currentAuthenticationRetries++ > MAX_RETRY_ATTEMPTS_ON_AUTH_ERROR) {
-                            log.error("Failed to login to node.", ex)
-                            throw ex
-                        }
+                        log.error("Failed to login to node.", ex)
+                        throw ex
                     }
                     is RPCException -> {
                         // Deliberately not logging full stack trace as it will be full of internal stacktraces.
@@ -204,7 +196,7 @@ class ReconnectingCordaRPCOps private constructor(
             Thread.sleep(retryInterval.toMillis())
             // TODO - make the exponential retry factor configurable.
             val nextRoundRobinIndex = (roundRobinIndex + 1) % nodeHostAndPorts.size
-            return establishConnectionWithRetry((retryInterval * 10) / 9, _currentAuthenticationRetries, nextRoundRobinIndex)
+            return establishConnectionWithRetry((retryInterval * 10) / 9, nextRoundRobinIndex)
         }
         override val proxy: CordaRPCOps
             get() = current.proxy

node/src/main/kotlin/net/corda/node/services/rpc/ArtemisRpcBroker.kt

@@ -15,6 +15,7 @@ import org.apache.activemq.artemis.core.server.ActiveMQServer
 import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl
 import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager
 import java.io.IOException
+import java.lang.RuntimeException
 import java.nio.file.Path
 import java.security.KeyStoreException
 import javax.security.auth.login.AppConfigurationEntry
@@ -60,6 +61,7 @@ class ArtemisRpcBroker internal constructor(
     override fun stop() {
         logger.debug("Artemis RPC broker is stopping.")
         server.stop(true)
+        securityManager.close()
         logger.debug("Artemis RPC broker is stopped.")
     }
 

node/src/main/kotlin/net/corda/node/services/rpc/RPCServer.kt

@@ -243,7 +243,6 @@ class RPCServer(
         reaperScheduledFuture?.cancel(false)
         rpcExecutor?.shutdownNow()
         reaperExecutor?.shutdownNow()
-        securityManager.close()
         sessionFactory?.close()
         observableMap.invalidateAll()
         reapSubscriptions()