From 205ce84033141da87d6589fcb43b85089552179f Mon Sep 17 00:00:00 2001
From: Stefano Franz <stefano.franz@r3.com>
Date: Fri, 14 Aug 2020 11:56:37 +0200
Subject: [PATCH] [EG-3461] removed dependency from tools.jar (#6631)

* removed dependency from tools.jar

I removed the log line in /node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt because I felt it was not so important
and I modified the checkpoint agent detection simply using a static field (I tested both with and without the checkpoint agent running and detection works correctly)

* move method to node-api to address review comments

Co-authored-by: Walter Oggioni <walter.oggioni@r3.com>
---
 .../nodeapi/internal/JVMAgentUtilities.kt     | 26 ++++++++++
 .../nodeapi/internal/ParseDebugPortTest.kt    | 32 ++++++++++++
 node/build.gradle                             |  3 --
 .../net/corda/node/internal/NodeStartup.kt    | 10 ++--
 .../node/services/rpc/CheckpointDumperImpl.kt | 50 +++++++++++--------
 .../net/corda/node/utilities/JVMAgentUtil.kt  | 25 ----------
 tools/checkpoint-agent/build.gradle           | 19 ++++---
 .../kotlin/net/corda/tools/CheckpointAgent.kt |  6 +++
 8 files changed, 108 insertions(+), 63 deletions(-)
 create mode 100644 node-api/src/main/kotlin/net/corda/nodeapi/internal/JVMAgentUtilities.kt
 create mode 100644 node-api/src/test/kotlin/net/corda/nodeapi/internal/ParseDebugPortTest.kt
 delete mode 100644 node/src/main/kotlin/net/corda/node/utilities/JVMAgentUtil.kt

diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/JVMAgentUtilities.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/JVMAgentUtilities.kt
new file mode 100644
index 0000000000..ed060ab4ac
--- /dev/null
+++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/JVMAgentUtilities.kt
@@ -0,0 +1,26 @@
+package net.corda.nodeapi.internal
+
+import net.corda.core.internal.VisibleForTesting
+
+class JVMAgentUtilities {
+
+    companion object {
+        @VisibleForTesting
+        @Suppress("NestedBlockDepth")
+        fun parseDebugPort(args: Iterable<String>): Short? {
+            val debugArgumentPrefix = "-agentlib:jdwp="
+            for (arg in args) {
+                if (arg.startsWith(debugArgumentPrefix)) {
+                    for (keyValuePair in arg.substring(debugArgumentPrefix.length + 1).split(",")) {
+                        val equal = keyValuePair.indexOf('=')
+                        if (equal >= 0 && keyValuePair.startsWith("address")) {
+                            val portBegin = (keyValuePair.lastIndexOf(':').takeUnless { it < 0 } ?: equal) + 1
+                            return keyValuePair.substring(portBegin).toShort()
+                        }
+                    }
+                }
+            }
+            return null
+        }
+    }
+}
\ No newline at end of file
diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/ParseDebugPortTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/ParseDebugPortTest.kt
new file mode 100644
index 0000000000..3fd71c5ef0
--- /dev/null
+++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/ParseDebugPortTest.kt
@@ -0,0 +1,32 @@
+package net.corda.nodeapi.internal
+
+import org.junit.Assert
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class ParseDebugPortTest(private val args: Iterable<String>,
+                         private val expectedPort: Short?,
+                         @Suppress("unused_parameter") description : String) {
+
+    companion object {
+        @JvmStatic
+        @Parameterized.Parameters(name = "{2}")
+        fun load() = arrayOf(
+                arrayOf(emptyList<String>(), null, "No arguments"),
+                arrayOf(listOf("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=1234"), 1234.toShort(), "Debug argument"),
+                arrayOf(listOf("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=0.0.0.0:7777"), 7777.toShort(), "Debug argument with bind address"),
+                arrayOf(listOf("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y"), null, "Debug argument without port"),
+                arrayOf(listOf("-version", "-Dmy.jvm.property=someValue"), null, "Unrelated arguments"),
+                arrayOf(listOf("-Dcapsule.jvm.args=\"-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=4321",
+                        "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=1234"), 1234.toShort(), "Debug argument and capsule arguments")
+        )
+    }
+
+    @Test(timeout = 10_000)
+    fun test() {
+        val port = JVMAgentUtilities.parseDebugPort(args)
+        Assert.assertEquals(expectedPort, port)
+    }
+}
\ No newline at end of file
diff --git a/node/build.gradle b/node/build.gradle
index 99fcb9760a..5f5eb08b4d 100644
--- a/node/build.gradle
+++ b/node/build.gradle
@@ -226,9 +226,6 @@ dependencies {
     // Adding native SSL library to allow using native SSL with Artemis and AMQP
     compile "io.netty:netty-tcnative-boringssl-static:$tcnative_version"
 
-    // Required by JVMAgentUtil (x-compatible java 8 & 11 agent lookup mechanism)
-    compile files("${System.properties['java.home']}/../lib/tools.jar")
-    
     // Byteman for runtime (termination) rules injection on the running node
     // Submission tool allowing to install rules on running nodes
     slowIntegrationTestCompile "org.jboss.byteman:byteman-submit:4.0.11"
diff --git a/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt b/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt
index 7400161c02..93353de002 100644
--- a/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt
+++ b/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt
@@ -28,8 +28,8 @@ import net.corda.node.internal.subcommands.ValidateConfigurationCli.Companion.lo
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.config.shouldStartLocalShell
 import net.corda.node.services.config.shouldStartSSHDaemon
-import net.corda.node.utilities.JVMAgentUtil.getJvmAgentProperties
 import net.corda.node.utilities.registration.NodeRegistrationException
+import net.corda.nodeapi.internal.JVMAgentUtilities
 import net.corda.nodeapi.internal.addShutdownHook
 import net.corda.nodeapi.internal.persistence.CouldNotCreateDataSourceException
 import net.corda.nodeapi.internal.persistence.DatabaseIncompatibleException
@@ -154,6 +154,8 @@ open class NodeStartup : NodeStartupLogging {
         const val LOGS_DIRECTORY_NAME = "logs"
         const val LOGS_CAN_BE_FOUND_IN_STRING = "Logs can be found in"
         const val ERROR_CODE_RESOURCE_LOCATION = "error-codes"
+
+
     }
 
     lateinit var cmdLineOptions: SharedNodeCmdLineOptions
@@ -270,10 +272,10 @@ open class NodeStartup : NodeStartupLogging {
         logger.info("VM ${info.vmName} ${info.vmVendor} ${info.vmVersion}")
         logger.info("Machine: ${lookupMachineNameAndMaybeWarn()}")
         logger.info("Working Directory: ${cmdLineOptions.baseDirectory}")
-        val agentProperties = getJvmAgentProperties(logger)
-        if (agentProperties.containsKey("sun.jdwp.listenerAddress")) {
-            logger.info("Debug port: ${agentProperties.getProperty("sun.jdwp.listenerAddress")}")
+        JVMAgentUtilities.parseDebugPort(info.inputArguments) ?.let {
+            logger.info("Debug port: $it")
         }
+
         var nodeStartedMessage = "Starting as node on ${conf.p2pAddress}"
         if (conf.extraNetworkMapKeys.isNotEmpty()) {
             nodeStartedMessage = "$nodeStartedMessage with additional Network Map keys ${conf.extraNetworkMapKeys.joinToString(prefix = "[", postfix = "]", separator = ", ")}"
diff --git a/node/src/main/kotlin/net/corda/node/services/rpc/CheckpointDumperImpl.kt b/node/src/main/kotlin/net/corda/node/services/rpc/CheckpointDumperImpl.kt
index 412ccd72a6..92b8771d91 100644
--- a/node/src/main/kotlin/net/corda/node/services/rpc/CheckpointDumperImpl.kt
+++ b/node/src/main/kotlin/net/corda/node/services/rpc/CheckpointDumperImpl.kt
@@ -33,7 +33,6 @@ import net.corda.core.flows.FlowSession
 import net.corda.core.flows.StateMachineRunId
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
-import net.corda.core.node.AppServiceHub.Companion.SERVICE_PRIORITY_NORMAL
 import net.corda.core.internal.FlowAsyncOperation
 import net.corda.core.internal.FlowIORequest
 import net.corda.core.internal.WaitForStateConsumption
@@ -43,6 +42,7 @@ import net.corda.core.internal.exists
 import net.corda.core.internal.objectOrNewInstance
 import net.corda.core.internal.outputStream
 import net.corda.core.internal.uncheckedCast
+import net.corda.core.node.AppServiceHub.Companion.SERVICE_PRIORITY_NORMAL
 import net.corda.core.node.ServiceHub
 import net.corda.core.serialization.SerializeAsToken
 import net.corda.core.serialization.SerializedBytes
@@ -54,9 +54,6 @@ import net.corda.core.utilities.NonEmptySet
 import net.corda.core.utilities.ProgressTracker
 import net.corda.core.utilities.Try
 import net.corda.core.utilities.contextLogger
-import net.corda.nodeapi.internal.lifecycle.NodeLifecycleEvent
-import net.corda.nodeapi.internal.lifecycle.NodeLifecycleObserver
-import net.corda.nodeapi.internal.lifecycle.NodeLifecycleObserver.Companion.reportSuccess
 import net.corda.node.internal.NodeStartup
 import net.corda.node.services.api.CheckpointStorage
 import net.corda.node.services.statemachine.Checkpoint
@@ -68,7 +65,9 @@ import net.corda.node.services.statemachine.FlowStateMachineImpl
 import net.corda.node.services.statemachine.SessionId
 import net.corda.node.services.statemachine.SessionState
 import net.corda.node.services.statemachine.SubFlow
-import net.corda.node.utilities.JVMAgentUtil.getJvmAgentProperties
+import net.corda.nodeapi.internal.lifecycle.NodeLifecycleEvent
+import net.corda.nodeapi.internal.lifecycle.NodeLifecycleObserver
+import net.corda.nodeapi.internal.lifecycle.NodeLifecycleObserver.Companion.reportSuccess
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.serialization.internal.CheckpointSerializeAsTokenContextImpl
 import net.corda.serialization.internal.withTokenContext
@@ -77,21 +76,24 @@ import java.time.Duration
 import java.time.Instant
 import java.time.ZoneOffset.UTC
 import java.time.format.DateTimeFormatter
-import java.util.UUID
+import java.util.*
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.atomic.AtomicInteger
 import java.util.zip.ZipEntry
 import java.util.zip.ZipOutputStream
+import kotlin.reflect.KProperty1
+import kotlin.reflect.full.companionObject
+import kotlin.reflect.full.memberProperties
 
 class CheckpointDumperImpl(private val checkpointStorage: CheckpointStorage, private val database: CordaPersistence,
-                                    private val serviceHub: ServiceHub, val baseDirectory: Path) : NodeLifecycleObserver {
+                           private val serviceHub: ServiceHub, val baseDirectory: Path) : NodeLifecycleObserver {
     companion object {
         internal val TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyyMMdd-HHmmss").withZone(UTC)
         private val log = contextLogger()
         private val DUMPABLE_CHECKPOINTS = setOf(
-            Checkpoint.FlowStatus.RUNNABLE,
-            Checkpoint.FlowStatus.HOSPITALIZED,
-            Checkpoint.FlowStatus.PAUSED
+                Checkpoint.FlowStatus.RUNNABLE,
+                Checkpoint.FlowStatus.HOSPITALIZED,
+                Checkpoint.FlowStatus.PAUSED
         )
     }
 
@@ -102,12 +104,10 @@ class CheckpointDumperImpl(private val checkpointStorage: CheckpointStorage, pri
     private lateinit var checkpointSerializationContext: CheckpointSerializationContext
     private lateinit var writer: ObjectWriter
 
-    private val isCheckpointAgentRunning by lazy {
-        checkpointAgentRunning()
-    }
+    private val isCheckpointAgentRunning by lazy(::checkpointAgentRunning)
 
     override fun update(nodeLifecycleEvent: NodeLifecycleEvent): Try<String> {
-        return when(nodeLifecycleEvent) {
+        return when (nodeLifecycleEvent) {
             is NodeLifecycleEvent.AfterNodeStart<*> -> Try.on {
                 checkpointSerializationContext = CheckpointSerializationDefaults.CHECKPOINT_CONTEXT.withTokenContext(
                         CheckpointSerializeAsTokenContextImpl(
@@ -190,13 +190,20 @@ class CheckpointDumperImpl(private val checkpointStorage: CheckpointStorage, pri
         }
     }
 
-    private fun checkpointAgentRunning(): Boolean {
-        val agentProperties = getJvmAgentProperties(log)
-        val pattern = "(.+)?checkpoint-agent(-.+)?\\.jar.*".toRegex()
-        return agentProperties.values.any { value ->
-            value is String && value.contains(pattern)
-        }
-    }
+    /**
+     * Note that this method dynamically uses [net.corda.tools.CheckpointAgent.running], make sure to keep it up to date with
+     * the checkpoint agent source code
+     */
+    private fun checkpointAgentRunning() = try {
+        javaClass.classLoader.loadClass("net.corda.tools.CheckpointAgent").kotlin.companionObject
+    } catch (e: ClassNotFoundException) {
+        null
+    }?.let { cls ->
+        @Suppress("UNCHECKED_CAST")
+        cls.memberProperties.find { it.name == "running"}
+                ?.let {it as KProperty1<Any, Boolean>}
+                ?.get(cls.objectInstance!!)
+    } ?: false
 
     private fun Checkpoint.toJson(id: UUID, now: Instant): CheckpointJson {
         val (fiber, flowLogic) = when (flowState) {
@@ -402,6 +409,7 @@ class CheckpointDumperImpl(private val checkpointStorage: CheckpointStorage, pri
     private interface FlowAsyncOperationMixin {
         @get:JsonIgnore
         val serviceHub: ServiceHub
+
         // [Any] used so this single mixin can serialize [FlowExternalOperation] and [FlowExternalAsyncOperation]
         @get:JsonUnwrapped
         val operation: Any
diff --git a/node/src/main/kotlin/net/corda/node/utilities/JVMAgentUtil.kt b/node/src/main/kotlin/net/corda/node/utilities/JVMAgentUtil.kt
deleted file mode 100644
index 8162be5d68..0000000000
--- a/node/src/main/kotlin/net/corda/node/utilities/JVMAgentUtil.kt
+++ /dev/null
@@ -1,25 +0,0 @@
-package net.corda.node.utilities
-
-import com.sun.tools.attach.VirtualMachine
-import org.slf4j.Logger
-import java.lang.management.ManagementFactory
-import java.util.*
-
-object JVMAgentUtil {
-    /**
-     * Utility to attach to own VM at run-time and obtain agent details.
-     * In Java 9 this requires setting the following run-time jvm flag: -Djdk.attach.allowAttachSelf=true
-     * This mechanism supersedes the usage of VMSupport which is not available from Java 9 onwards.
-     */
-    fun getJvmAgentProperties(log: Logger): Properties {
-        val jvmPid = ManagementFactory.getRuntimeMXBean().name.substringBefore('@')
-        return try {
-            val vm = VirtualMachine.attach(jvmPid)
-            return vm.agentProperties
-        } catch (e: Throwable) {
-            log.warn("Unable to determine whether agent is running: ${e.message}.\n" +
-                     "You may need to pass in -Djdk.attach.allowAttachSelf=true if running on a Java 9 or later VM")
-            Properties()
-        }
-    }
-}
\ No newline at end of file
diff --git a/tools/checkpoint-agent/build.gradle b/tools/checkpoint-agent/build.gradle
index 33fc683eb6..228a6de595 100644
--- a/tools/checkpoint-agent/build.gradle
+++ b/tools/checkpoint-agent/build.gradle
@@ -31,23 +31,22 @@ apply plugin: 'com.jfrog.artifactory'
 description 'A javaagent to allow hooking into Kryo checkpoints'
 
 dependencies {
-    compile "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
-    compile "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
-    compile "javassist:javassist:$javaassist_version"
-    compile "com.esotericsoftware:kryo:$kryo_version"
-    compile "co.paralleluniverse:quasar-core:$quasar_version"
+    compileOnly "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version"
+    compileOnly "org.jetbrains.kotlin:kotlin-reflect:$kotlin_version"
+    compileOnly "javassist:javassist:$javaassist_version"
+    compileOnly "com.esotericsoftware:kryo:$kryo_version"
+    compileOnly "co.paralleluniverse:quasar-core:$quasar_version"
 
-    compile (project(':core')) {
+    compileOnly (project(':core')) {
         transitive = false
     }
 
     // Unit testing helpers.
-    compile "org.junit.jupiter:junit-jupiter-api:${junit_jupiter_version}"
-    compile "junit:junit:$junit_version"
+    testCompile "org.junit.jupiter:junit-jupiter-api:${junit_jupiter_version}"
+    testCompile "junit:junit:$junit_version"
 
     // SLF4J: commons-logging bindings for a SLF4J back end
-    compile "org.slf4j:jcl-over-slf4j:$slf4j_version"
-    compile "org.slf4j:slf4j-api:$slf4j_version"
+    compileOnly "org.slf4j:slf4j-api:$slf4j_version"
 }
 
 jar {
diff --git a/tools/checkpoint-agent/src/main/kotlin/net/corda/tools/CheckpointAgent.kt b/tools/checkpoint-agent/src/main/kotlin/net/corda/tools/CheckpointAgent.kt
index d7169ee967..9fb3aa2b84 100644
--- a/tools/checkpoint-agent/src/main/kotlin/net/corda/tools/CheckpointAgent.kt
+++ b/tools/checkpoint-agent/src/main/kotlin/net/corda/tools/CheckpointAgent.kt
@@ -51,8 +51,14 @@ class CheckpointAgent {
             LoggerFactory.getLogger("CheckpointAgent")
         }
 
+        val running by lazy {
+            premainExecuted
+        }
+        private var premainExecuted = false
+
         @JvmStatic
         fun premain(argumentsString: String?, instrumentation: Instrumentation) {
+            premainExecuted = true
             parseArguments(argumentsString)
             instrumentation.addTransformer(CheckpointHook)
         }