Fix re-using process for debugging

Each time we disconnect the debugger, the child process opens a new port that we must read to connect a new debugger

Author: adpi2

compiler/test/dotty/tools/debug/DebugTests.scala

@@ -7,14 +7,15 @@ import dotty.tools.io.JFile
 import dotty.tools.vulpix.*
 import org.junit.Test
 
+import java.util.concurrent.TimeoutException
 import scala.concurrent.duration.*
 import scala.util.control.NonFatal
 
 class DebugTests:
   import DebugTests.*
   @Test def debug: Unit =
     implicit val testGroup: TestGroup = TestGroup("debug")
-    // compileFile("tests/debug/eval-static-fields.scala", TestConfiguration.defaultOptions).checkDebug()
+    // compileFile("tests/debug/eval-private-members-in-parent.scala", TestConfiguration.defaultOptions).checkDebug()
     compileFilesInDir("tests/debug", TestConfiguration.defaultOptions).checkDebug()
 
 object DebugTests extends ParallelTesting:
@@ -49,21 +50,24 @@ object DebugTests extends ParallelTesting:
         val debugSteps = DebugStepAssert.parseCheckFile(checkFile)
         val expressionEvaluator =
           ExpressionEvaluator(testSource.sourceFiles, testSource.flags, testSource.runClassPath, testSource.outDir)
-        try
-          val status = debugMain(testSource.runClassPath): debuggee =>
-            val debugger = Debugger(debuggee.jdiPort, expressionEvaluator, maxDuration/* , verbose = true */)
-            // configure the breakpoints before starting the debuggee
-            val breakpoints = debugSteps.map(_.step).collect { case b: DebugStep.Break => b }
-            for b <- breakpoints do debugger.configureBreakpoint(b.className, b.line)
-            try
-              debuggee.launch()
-              playDebugSteps(debugger, debugSteps/* , verbose = true */)
-            finally
-              // stop debugger to let debuggee terminate its execution
-              debugger.dispose()
-          reportDebuggeeStatus(testSource, status)
+        try debugMain(testSource.runClassPath): debuggee =>
+          val jdiPort = debuggee.readJdiPort()
+          val debugger = Debugger(jdiPort, expressionEvaluator, maxDuration/* , verbose = true */)
+          // configure the breakpoints before starting the debuggee
+          val breakpoints = debugSteps.map(_.step).collect { case b: DebugStep.Break => b }.distinct
+          for b <- breakpoints do debugger.configureBreakpoint(b.className, b.line)
+          try
+            debuggee.launch()
+            playDebugSteps(debugger, debugSteps/* , verbose = true */)
+            val status = debuggee.exit()
+            reportDebuggeeStatus(testSource, status)
+          finally
+            // closing the debugger must be done at the very end so that the
+            // 'Listening for transport dt_socket at address: <port>' message is ready to be read
+            // by the next DebugTest
+            debugger.dispose()
         catch case DebugStepException(message, location) =>
-          echo(s"\nDebug step failed: $location\n" + message)
+          echo(s"\n[error] Debug step failed: $location\n" + message)
           failTestSource(testSource)
     end verifyDebug
 
@@ -77,33 +81,41 @@ object DebugTests extends ParallelTesting:
        */
       var thread: ThreadReference = null
       def location = thread.frame(0).location
+      def continueIfPaused(): Unit =
+        if thread != null then
+          debugger.continue(thread)
+          thread = null
 
       for case step <- steps do
         import DebugStep.*
-        step match
+        try step match
           case DebugStepAssert(Break(className, line), assert) =>
-            // continue if paused
-            if thread != null then
-              debugger.continue(thread)
-              thread = null
+            continueIfPaused()
             thread = debugger.break()
-            if verbose then println(s"break ${location.declaringType.name} ${location.lineNumber}")
+            if verbose then
+              println(s"break $location ${location.method.name}")
             assert(location)
           case DebugStepAssert(Next, assert) =>
             thread = debugger.next(thread)
-            if verbose then println(s"next ${location.lineNumber}")
+            if verbose then println(s"next $location ${location.method.name}")
             assert(location)
           case DebugStepAssert(Step, assert) =>
             thread = debugger.step(thread)
-            if verbose then println(s"step ${location.lineNumber}")
+            if verbose then println(s"step $location ${location.method.name}")
             assert(location)
           case DebugStepAssert(Eval(expr), assert) =>
-            val result =
-              try debugger.evaluate(expr, thread)
-              catch case NonFatal(cause) =>
-                throw new Exception(s"Evaluation of $expr failed", cause)
-            if verbose then println(s"eval $expr $result")
+            if verbose then println(s"eval $expr")
+            val result = debugger.evaluate(expr, thread)
+            if verbose then println(result.fold("error " + _, "result " + _))
             assert(result)
+        catch
+          case _: TimeoutException => throw new DebugStepException("Timeout", step.location)
+          case e: DebugStepException => throw e
+          case NonFatal(e) =>
+            throw new Exception(s"Debug step failed unexpectedly: ${step.location}", e)
+      end for
+      // let the debuggee finish its execution
+      continueIfPaused()
     end playDebugSteps
 
     private def reportDebuggeeStatus(testSource: TestSource, status: Status): Unit =

compiler/test/dotty/tools/vulpix/RunnerOrchestration.scala

@@ -59,16 +59,19 @@ trait RunnerOrchestration {
   def runMain(classPath: String, toolArgs: ToolArgs)(implicit summaryReport: SummaryReporting): Status =
     monitor.runMain(classPath)
 
+  /** Each method of Debuggee can be called only once, in the order of definition.*/
   trait Debuggee:
-    /** the jdi port to connect the debugger */
-    def jdiPort: Int
+    /** read the jdi port to connect the debugger */
+    def readJdiPort(): Int
     /** start the main method in the background */
     def launch(): Unit
+    /** wait until the end of the main method */
+    def exit(): Status
 
   /** Provide a Debuggee for debugging the Test class's main method
-   * @param f the debugging flow: set breakpoints, launch main class, pause, step, evaluate etc
+   * @param f the debugging flow: set breakpoints, launch main class, pause, step, evaluate, exit etc
   */
-  def debugMain(classPath: String)(f: Debuggee => Unit)(implicit summaryReport: SummaryReporting): Status =
+  def debugMain(classPath: String)(f: Debuggee => Unit)(implicit summaryReport: SummaryReporting): Unit =
     assert(debugMode, "debugMode is disabled")
     monitor.debugMain(classPath)(f)
 
@@ -92,14 +95,31 @@ trait RunnerOrchestration {
     def runMain(classPath: String)(implicit summaryReport: SummaryReporting): Status =
       withRunner(_.runMain(classPath))
 
-    def debugMain(classPath: String)(f: Debuggee => Unit)(implicit summaryReport: SummaryReporting): Status =
+    def debugMain(classPath: String)(f: Debuggee => Unit)(implicit summaryReport: SummaryReporting): Unit =
       withRunner(_.debugMain(classPath)(f))
 
-    // A JVM process and its JDI port for debugging, if debugMode is enabled.
-    private class RunnerProcess(p: Process, val jdiPort: Option[Int]):
-      val stdout = new BufferedReader(new InputStreamReader(p.getInputStream(), StandardCharsets.UTF_8))
-      val stdin = new PrintStream(p.getOutputStream(), /* autoFlush = */ true)
+    private class RunnerProcess(p: Process):
+      private val stdout = new BufferedReader(new InputStreamReader(p.getInputStream(), StandardCharsets.UTF_8))
+      private val stdin = new PrintStream(p.getOutputStream(), /* autoFlush = */ true)
+
+      def readLine(): String =
+        stdout.readLine() match
+          case s"Listening for transport dt_socket at address: $port" =>
+            throw new IOException(
+              s"Unexpected transport dt_socket message." +
+              " The port is going to be lost and no debugger will be able to connect."
+            )
+          case line => line
+
+      def printLine(line: String): Unit = stdin.println(line)
+
+      def getJdiPort(): Int =
+        stdout.readLine() match
+          case s"Listening for transport dt_socket at address: $port" => port.toInt
+          case line => throw new IOException(s"Failed getting JDI port of child JVM: got $line")
+
       export p.{exitValue, isAlive, destroy}
+    end RunnerProcess
 
     private class Runner(private var process: RunnerProcess):
 
@@ -111,7 +131,7 @@ trait RunnerOrchestration {
        */
       def isAlive: Boolean =
         try { process.exitValue(); false }
-        catch { case _: IllegalThreadStateException => true }
+        catch case _: IllegalThreadStateException => true
 
       /** Destroys the underlying process and kills IO streams */
       def kill(): Unit =
@@ -123,51 +143,50 @@ trait RunnerOrchestration {
         assert(process ne null, "Runner was killed and then reused without setting a new process")
         awaitStatusOrRespawn(startMain(classPath))
 
-      def debugMain(classPath: String)(f: Debuggee => Unit): Status =
+      def debugMain(classPath: String)(f: Debuggee => Unit): Unit =
         assert(process ne null, "Runner was killed and then reused without setting a new process")
-        assert(process.jdiPort.isDefined, "Runner has not been started in debug mode")
 
-        var mainFuture: Future[Status] = null
         val debuggee = new Debuggee:
-          def jdiPort: Int = process.jdiPort.get
-          def launch(): Unit =
-            mainFuture = startMain(classPath)
+          var mainFuture: Future[Status] = null
+          def readJdiPort(): Int = process.getJdiPort()
+          def launch(): Unit = mainFuture = startMain(classPath)
+          def exit(): Status =
+            awaitStatusOrRespawn(mainFuture)
 
         try f(debuggee)
-        catch case debugFailure: Throwable =>
-          if mainFuture != null then awaitStatusOrRespawn(mainFuture)
-          throw debugFailure
-
-        assert(mainFuture ne null, "main method not started by debugger")
-        awaitStatusOrRespawn(mainFuture)
+        catch case e: Throwable =>
+          // if debugging failed it is safer to respawn a new process
+          respawn()
+          throw e
       end debugMain
 
       private def startMain(classPath: String): Future[Status] =
         // pass classpath to running process
-        process.stdin.println(classPath)
+        process.printLine(classPath)
 
         // Create a future reading the object:
         Future:
           val sb = new StringBuilder
 
-          var childOutput: String = process.stdout.readLine()
+          var childOutput: String = process.readLine()
 
           // Discard all messages until the test starts
           while (childOutput != ChildJVMMain.MessageStart && childOutput != null)
-            childOutput = process.stdout.readLine()
-          childOutput = process.stdout.readLine()
+            childOutput = process.readLine()
+          childOutput = process.readLine()
 
           while childOutput != ChildJVMMain.MessageEnd && childOutput != null do
             sb.append(childOutput).append(System.lineSeparator)
-            childOutput = process.stdout.readLine()
+            childOutput = process.readLine()
 
-          if (process.isAlive() && childOutput != null) Success(sb.toString)
+          if process.isAlive() && childOutput != null then Success(sb.toString)
           else Failure(sb.toString)
       end startMain
 
       // wait status of the main class execution, respawn if failure or timeout
       private def awaitStatusOrRespawn(future: Future[Status]): Status =
-        val status = try Await.result(future, maxDuration)
+        val status =
+          try Await.result(future, maxDuration)
           catch case _: TimeoutException => Timeout
         // handle failures
         status match
@@ -178,13 +197,13 @@ trait RunnerOrchestration {
       // Makes the encapsulating RunnerMonitor spawn a new runner
       private def respawn(): Unit =
         process.destroy()
-        process = createProcess
+        process = createProcess()
     end Runner
 
     /** Create a process which has the classpath of the `ChildJVMMain` and the
      *  scala library.
      */
-    private def createProcess: RunnerProcess = {
+    private def createProcess(): RunnerProcess =
       val url = classOf[ChildJVMMain].getProtectionDomain.getCodeSource.getLocation
       val cp = Paths.get(url.toURI).toString + JFile.pathSeparator + Properties.scalaLibrary
       val javaBin = Paths.get(sys.props("java.home"), "bin", "java").toString
@@ -196,15 +215,7 @@ trait RunnerOrchestration {
         .redirectInput(ProcessBuilder.Redirect.PIPE)
         .redirectOutput(ProcessBuilder.Redirect.PIPE)
         .start()
-
-      val jdiPort = Option.when(debugMode):
-        val reader = new BufferedReader(new InputStreamReader(process.getInputStream, StandardCharsets.UTF_8))
-        reader.readLine() match
-          case s"Listening for transport dt_socket at address: $port" => port.toInt
-          case line => throw new IOException(s"Failed getting JDI port of child JVM: got $line")
-
-      RunnerProcess(process, jdiPort)
-    }
+      RunnerProcess(process)
 
     private val freeRunners = mutable.Queue.empty[Runner]
     private val busyRunners = mutable.Set.empty[Runner]
@@ -213,7 +224,7 @@ trait RunnerOrchestration {
       while (freeRunners.isEmpty && busyRunners.size >= numberOfSlaves) wait()
 
       val runner =
-        if (freeRunners.isEmpty) new Runner(createProcess)
+        if (freeRunners.isEmpty) new Runner(createProcess())
         else freeRunners.dequeue()
       busyRunners += runner