Skip to content

feat: support cwd and env variable using sun jdi #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions adapter/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ dependencies {
implementation 'com.github.fwcd.kotlin-language-server:shared:229c762a4d75304d21eba6d8e1231ed949247629'
// The Java Debug Interface classes (com.sun.jdi.*)
implementation files("${System.properties['java.home']}/../lib/tools.jar")
// For CommandLineUtils.translateCommandLine
implementation 'org.codehaus.plexus:plexus-utils:3.3.0'
Copy link
Owner

@fwcd fwcd Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, I would like to avoid introducing a new dependency here. For splitting the command line args, I think just splitting by spaces would be fine for now.

testImplementation 'junit:junit:4.12'
testImplementation 'org.hamcrest:hamcrest-all:1.3'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ class KotlinDebugAdapter(
override fun launch(args: Map<String, Any>) = launcherAsync.execute {
performInitialization()

LOG.debug("launch args: $args")

val projectRoot = (args["projectRoot"] as? String)?.let { Paths.get(it) }
?: throw missingRequestArgument("launch", "projectRoot")

Expand All @@ -100,13 +102,21 @@ class KotlinDebugAdapter(

val vmArguments = (args["vmArguments"] as? String) ?: ""

var cwd = (args["cwd"] as? String).let { if(it.isNullOrBlank()) projectRoot else Paths.get(it) }

// Cast from com.google.gson.internal.LinkedTreeMap
@Suppress("UNCHECKED_CAST")
var envs = args["envs"] as? Map<String, String> ?: mapOf()

setupCommonInitializationParams(args)

val config = LaunchConfiguration(
debugClassPathResolver(listOf(projectRoot)).classpathOrEmpty,
mainClass,
projectRoot,
vmArguments
vmArguments,
cwd,
envs
)
debuggee = launcher.launch(
config,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@ class LaunchConfiguration(
val classpath: Set<Path>,
val mainClass: String,
val projectRoot: Path,
val vmArguments: String = ""
val vmArguments: String = "",
val cwd: Path = projectRoot,
val envs: Map<String, String> = mapOf()
)
24 changes: 7 additions & 17 deletions adapter/src/main/kotlin/org/javacs/ktda/jdi/launch/JDILauncher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import org.javacs.kt.LOG
import org.javacs.ktda.core.launch.DebugLauncher
import org.javacs.ktda.core.launch.LaunchConfiguration
import org.javacs.ktda.core.launch.AttachConfiguration
import org.javacs.ktda.core.Debuggee
import org.javacs.ktda.core.DebugContext
import org.javacs.ktda.util.KotlinDAException
import org.javacs.ktda.jdi.JDIDebuggee
Expand All @@ -16,14 +15,11 @@ import com.sun.jdi.connect.AttachingConnector
import java.io.File
import java.nio.file.Path
import java.nio.file.Files
import java.net.URLEncoder
import java.net.URLDecoder
import java.nio.charset.StandardCharsets
import java.util.stream.Collectors
import org.javacs.kt.LOG

class JDILauncher(
private val attachTimeout: Int = 50,
private val vmArguments: String? = null,
private val modulePaths: String? = null
) : DebugLauncher {
private val vmManager: VirtualMachineManager
Expand Down Expand Up @@ -57,6 +53,8 @@ class JDILauncher(
args["suspend"]!!.setValue("true")
args["options"]!!.setValue(formatOptions(config))
args["main"]!!.setValue(formatMainClass(config))
args["cwd"]!!.setValue(config.cwd.toAbsolutePath().toString())
args["envs"]!!.setValue(KDACommandLineLauncher.urlEncode(config.envs.map { "${it.key}=${it.value}" }) ?: "")
}

private fun createAttachArgs(config: AttachConfiguration, connector: Connector): Map<String, Connector.Argument> = connector.defaultArguments()
Expand All @@ -70,9 +68,9 @@ class JDILauncher(
.let { it.find { it.name() == "com.sun.jdi.SocketAttach" } ?: it.firstOrNull() }
?: throw KotlinDAException("Could not find an attaching connector (for a new debuggee VM)")

private fun createLaunchConnector(): LaunchingConnector = vmManager.launchingConnectors()
// Workaround for JDK 11+ where the first launcher (RawCommandLineLauncher) does not properly support args
.let { it.find { it.javaClass.name == "com.sun.tools.jdi.SunCommandLineLauncher" } ?: it.firstOrNull() }
private fun createLaunchConnector(): LaunchingConnector = vmManager.launchingConnectors().also { LOG.debug("connectors: $it") }
// Using our own connector to support cwd and envs
.let { it.find { it.name() == KDACommandLineLauncher::class.java.name } ?: it.firstOrNull() }
?: throw KotlinDAException("Could not find a launching connector (for a new debuggee VM)")

private fun sourcesRootsOf(projectRoot: Path): Set<Path> = projectRoot.resolve("src")
Expand Down Expand Up @@ -100,13 +98,5 @@ class JDILauncher(
private fun formatClasspath(config: LaunchConfiguration): String = config.classpath
.map { it.toAbsolutePath().toString() }
.reduce { prev, next -> "$prev${File.pathSeparatorChar}$next" }

private fun urlEncode(arg: Collection<String>?) = arg
?.map { URLEncoder.encode(it, StandardCharsets.UTF_8.name()) }
?.reduce { a, b -> "$a\n$b" }

private fun urlDecode(arg: String?) = arg
?.split("\n")
?.map { URLDecoder.decode(it, StandardCharsets.UTF_8.name()) }
?.toList()

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
package org.javacs.ktda.jdi.launch

import com.sun.jdi.Bootstrap
import com.sun.jdi.VirtualMachine
import com.sun.jdi.connect.*
import com.sun.jdi.connect.spi.Connection
import com.sun.jdi.connect.spi.TransportService
import org.codehaus.plexus.util.cli.CommandLineUtils
import org.javacs.kt.LOG
import java.io.File
import java.io.IOException
import java.net.URLDecoder
import java.net.URLEncoder
import java.nio.charset.StandardCharsets
import java.nio.file.Files
import java.nio.file.Paths
import java.util.*
import java.util.concurrent.Callable
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit

internal const val ARG_HOME = "home"
internal const val ARG_OPTIONS = "options"
internal const val ARG_MAIN = "main"
internal const val ARG_SUSPEND = "suspend"
internal const val ARG_QUOTE = "quote"
internal const val ARG_VM_EXEC = "vmexec"
internal const val ARG_CWD = "cwd"
internal const val ARG_ENVS = "envs"

/**
* A custom LaunchingConnector that supports cwd and env variables
*/
open class KDACommandLineLauncher : LaunchingConnector {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason to make this open, e.g. do we need to do so for interop with Java/JDI? If not, I would suggest removing the modifier.


protected val defaultArguments = mutableMapOf<String, Connector.Argument>()

/**
* We only support SocketTransportService
*/
protected var transportService : TransportService? = null
protected var transport = Transport { "dt_socket" }

companion object {

fun urlEncode(arg: Collection<String>?) = arg
?.map { URLEncoder.encode(it, StandardCharsets.UTF_8.name()) }
?.fold("") { a, b -> "$a\n$b" }

fun urlDecode(arg: String?) = arg
?.trim('\n')
?.split("\n")
?.map { URLDecoder.decode(it, StandardCharsets.UTF_8.name()) }
?.toList()
}

constructor() : super() {

defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))

defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")

defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)

defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")

defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")

defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")

defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")

defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")
Comment on lines +59 to +73
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: Please remove the blank lines inbetween.

Suggested change
defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))
defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")
defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)
defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")
defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")
defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")
defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")
defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")
defaultArguments[ARG_HOME] = StringArgument(ARG_HOME, description = "Java home", value = System.getProperty("java.home"))
defaultArguments[ARG_OPTIONS] = StringArgument(ARG_OPTIONS, description = "Jvm arguments")
defaultArguments[ARG_MAIN] = StringArgument(ARG_MAIN, description = "Main class name and parameters", mustSpecify = true)
defaultArguments[ARG_SUSPEND] = StringArgument(ARG_SUSPEND, description = "Whether launch the debugee in suspend mode", value = "true")
defaultArguments[ARG_QUOTE] = StringArgument(ARG_QUOTE, description = "Quote char", value = "\"")
defaultArguments[ARG_VM_EXEC] = StringArgument(ARG_VM_EXEC, description = "The java exec", value = "java")
defaultArguments[ARG_CWD] = StringArgument(ARG_CWD, description = "Current working directory")
defaultArguments[ARG_ENVS] = StringArgument(ARG_ENVS, description = "Environment variables")


// Load TransportService 's implementation
transportService = Class.forName("com.sun.tools.jdi.SocketTransportService").getDeclaredConstructor().newInstance() as TransportService

if(transportService == null){
throw IllegalStateException("Failed to load com.sun.tools.jdi.SocketTransportService")
}

}

override fun name(): String {
return this.javaClass.name
}

override fun description(): String {
return "A custom launcher supporting cwd and env variables"
}

override fun defaultArguments(): Map<String, Connector.Argument> {
return this.defaultArguments
}

override fun toString(): String {
return name()
}

override fun transport(): Transport {
return transport
}

protected fun getOrDefault(arguments: Map<String, Connector.Argument>, argName: String): String {
return arguments[argName]?.value() ?: defaultArguments[argName]?.value() ?: ""
}

/**
* A customized method to launch the vm and connect to it, supporting cwd and env variables
*/
@Throws(IOException::class, IllegalConnectorArgumentsException::class, VMStartException::class)
override fun launch(arguments: Map<String, Connector.Argument>): VirtualMachine {
val vm: VirtualMachine

val home = getOrDefault(arguments, ARG_HOME)
val options = getOrDefault(arguments, ARG_OPTIONS)
val main = getOrDefault(arguments, ARG_MAIN)
val suspend = getOrDefault(arguments, ARG_SUSPEND).toBoolean()
val quote = getOrDefault(arguments, ARG_QUOTE)
var exe = getOrDefault(arguments, ARG_VM_EXEC)
val cwd = getOrDefault(arguments, ARG_CWD)
val envs = urlDecode(getOrDefault(arguments, ARG_ENVS))?.toTypedArray()

check(quote.length == 1) {"Invalid length for $ARG_QUOTE: $quote"}
check(!options.contains("-Djava.compiler=") ||
options.toLowerCase().contains("-djava.compiler=none")) { "Cannot debug with a JIT compiler. $ARG_OPTIONS: $options"}

val listenKey = transportService?.startListening() ?: throw IllegalStateException("Failed to do transportService.startListening()")
val address = listenKey.address()

try {
val command = StringBuilder()

exe = if (home.isNotEmpty()) Paths.get(home, "bin", exe).toString() else exe
command.append(wrapWhitespace(exe))

command.append(" $options")

//debug options
command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")

command.append(" $main")
Comment on lines +135 to +142
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I think it looks a bit nicer with less whitespace.

Suggested change
command.append(wrapWhitespace(exe))
command.append(" $options")
//debug options
command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")
command.append(" $main")
command.append(wrapWhitespace(exe))
command.append(" $options")
// debug options
command.append(" -agentlib:jdwp=transport=${transport.name()},address=$address,server=n,suspend=${if (suspend) 'y' else 'n'}")
command.append(" $main")


LOG.debug("command before tokenize: $command")

vm = launch(commandArray = CommandLineUtils.translateCommandline(command.toString()), listenKey = listenKey,
ts = transportService!!, cwd = cwd, envs = envs
)

} finally {
transportService?.stopListening(listenKey)
}
return vm
}

internal fun wrapWhitespace(str: String): String {
return if(str.contains(' ')) "\"$str\" " else str
}

@Throws(IOException::class, VMStartException::class)
fun launch(commandArray: Array<String>,
listenKey: TransportService.ListenKey,
ts: TransportService, cwd: String, envs: Array<String>? = null): VirtualMachine {
Comment on lines +161 to +163
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would read better if we moved the parameters to individual lines:

Suggested change
fun launch(commandArray: Array<String>,
listenKey: TransportService.ListenKey,
ts: TransportService, cwd: String, envs: Array<String>? = null): VirtualMachine {
fun launch(
commandArray: Array<String>,
listenKey: TransportService.ListenKey,
ts: TransportService,
cwd: String,
envs: Array<String>? = null
): VirtualMachine {


val (connection, process) = launchAndConnect(commandArray, listenKey, ts, cwd = cwd, envs = envs)

return Bootstrap.virtualMachineManager().createVirtualMachine(connection,
process)
Comment on lines +167 to +168
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have a bunch of longer lines, I think not breaking would be fine here.

Suggested change
return Bootstrap.virtualMachineManager().createVirtualMachine(connection,
process)
return Bootstrap.virtualMachineManager().createVirtualMachine(connection, process)

}


/**
* launch the command, connect to transportService, and returns the connection / process pair
*/
protected fun launchAndConnect(commandArray: Array<String>, listenKey: TransportService.ListenKey,
ts: TransportService, cwd: String = "", envs: Array<String>? = null): Pair<Connection, Process>{
Comment on lines +175 to +176
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected fun launchAndConnect(commandArray: Array<String>, listenKey: TransportService.ListenKey,
ts: TransportService, cwd: String = "", envs: Array<String>? = null): Pair<Connection, Process>{
protected fun launchAndConnect(
commandArray: Array<String>,
listenKey: TransportService.ListenKey,
ts: TransportService,
cwd: String = "",
envs: Array<String>? = null
): Pair<Connection, Process> {


val dir = if(cwd.isNotBlank() && Files.isDirectory(Paths.get(cwd))) File(cwd) else null

var threadCount = 0

val executors = Executors.newFixedThreadPool(2) { Thread(it, "${this.javaClass.simpleName}-${threadCount++}") }
val process = Runtime.getRuntime().exec(commandArray, envs, dir)

val connectionTask: Callable<Any> = Callable { ts.accept(listenKey, 0,0).also { LOG.debug("ts.accept invoked") } }
val exitCodeTask: Callable<Any> = Callable { process.waitFor().also { LOG.debug("process.waitFor invoked") } }

try {
when (val result = executors.invokeAny(listOf(connectionTask, exitCodeTask))) {
// successfully connected to transport service
is Connection -> return Pair(result, process)

// cmd exited before connection. some thing wrong
is Int -> throw VMStartException(
"VM initialization failed. exit code: ${process?.exitValue()}, cmd: $commandArray", process)

// should never occur
else -> throw IllegalStateException("Unknown result: $result")
}
} finally {
// release the executors. no longer needed.
executors.shutdown()
executors.awaitTermination(1, TimeUnit.SECONDS)
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.javacs.ktda.jdi.launch

import com.sun.jdi.connect.Connector

/**
* An implementation to Connector.Argument, used for arguments to launch a LaunchingConnector
*/
class StringArgument constructor(private val name: String, private val description: String = "", private val label: String = name,
private var value:String = "", private val mustSpecify: Boolean = false) : Connector.Argument {
Comment on lines +8 to +9
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we omit the constructor here, since it is the primary constructor?

Suggested change
class StringArgument constructor(private val name: String, private val description: String = "", private val label: String = name,
private var value:String = "", private val mustSpecify: Boolean = false) : Connector.Argument {
class StringArgument(
private val name: String,
private val description: String = "",
private val label: String = name,
private var value:String = "",
private val mustSpecify: Boolean = false
) : Connector.Argument {


override fun name(): String {
return name
}

override fun description(): String {
return description
}

override fun label(): String {
return label
}

override fun mustSpecify(): Boolean {
return mustSpecify
}

override fun value(): String {
return value
}

override fun setValue(value: String){
this.value = value
}

override fun isValid(value: String): Boolean{
return true
}
override fun toString(): String {
return value
}


}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if you could make sure that all edited files have trailing newlines (by Unix convention). Most editors should be configurable to do so, e.g. in VSCode you can set

"files.insertFinalNewline": true

in your settings.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class JDIVariable(
}

private fun arrayElementsOf(jdiValue: ArrayReference): List<VariableTreeNode> = jdiValue.values
.mapIndexed { i, it -> JDIVariable(i.toString(), it) }
.mapIndexed { i, it -> JDIVariable(i.toString(), it) }

private fun fieldsOf(jdiValue: ObjectReference, jdiType: ReferenceType) = jdiType.allFields()
.map { JDIVariable(it.name(), jdiValue.getValue(it), jdiType) }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.javacs.ktda.jdi.launch.KDACommandLineLauncher
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import org.hamcrest.Matchers.equalTo
abstract class DebugAdapterTestFixture(
relativeWorkspaceRoot: String,
private val mainClass: String,
private val vmArguments: String = ""
private val vmArguments: String = "",
private val cwd: String = "",
private val envs: Map<String, String> = mapOf()
) : IDebugProtocolClient {
val absoluteWorkspaceRoot: Path = Paths.get(DebugAdapterTestFixture::class.java.getResource("/Anchor.txt").toURI()).parent.resolve(relativeWorkspaceRoot)
lateinit var debugAdapter: KotlinDebugAdapter
Expand Down Expand Up @@ -60,7 +62,9 @@ abstract class DebugAdapterTestFixture(
debugAdapter.launch(mapOf(
"projectRoot" to absoluteWorkspaceRoot.toString(),
"mainClass" to mainClass,
"vmArguments" to vmArguments
"vmArguments" to vmArguments,
"cwd" to cwd,
"envs" to envs
)).join()
println("Launched")
}
Expand Down
Loading