Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8353955

nsk/jdi tests should be fixed to not always require includevirtualthreads=y

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Unresolved
    • Icon: P3 P3
    • 25
    • 25
    • core-svc

      includevirtualthreads=y is a debug agent option that forces the debug agent to keep track of all created virtual threads, and return them when JDI vm.allClasses() is called. Some tests require this functionality in order to discover debuggee threads. However, always using it eliminates includevirtualthreads=n testing coverage, which is more important since IDEs normally don't run with includevirtualthreads=y. Furthermore, it will negate the changes being planned for JDK-8282441 since they will be disabled when using includevirtualthreads=y. Thus we won't have sufficient testing for testing JDK-8282441 changes.

      There are about 1200 nsk/jdi tests. I tried running them with includevirtualthreads=n, and about 350 failed. I think it will be pretty easy to make is so only these 350 tests are run with includevirtualthreads=y. The following code in Binder.java is what enables this option:

              // This flag is needed so VirtualMachine.allThreads() includes known vthreads.
              arg = (Connector.StringArgument) arguments.get("includevirtualthreads");
              arg.setValue("y");

      Every test creates a Binder instance. It would be easy to default this option to being off and have tests that require it to be on either pass an extra flag to the constructor, or call a method to turn it on after creating the Binder instance. It might also be possible to pass a special argument on the @run command, although I find this type of argument passing very hard to follow, so it's not clear to me how to detect the argument in Binder.java.

      We could also possibly find a different way for tests to find debuggee threads rather than rely on vm.allThreads(). One idea I had is using a global breakpoint to detect when new debugge threads are started. The debuggee thread will need to call a method like "registerNewThread()", which will have a global breakpoint set on it. When the bp event is received, the handler can store the ThreadReference in a location where methods like Debugee.threadByName() and Debugee.threadbyNameOrThrow() can find it.

      This is pretty easy to do for tests that use TestDebuggerType1, since it already has an EventHandler thread that can catch the BreakpointEvent. See setCommunicationBreakpoint() as an example on how to setup the breakpoint. However, not that many tests are of type TestDebuggerType1, and this approach may be harder to retrofit into TestDebuggerType2 tests, which don't use an EventHandler thread. There are also many places were vm.allThreads() is called. We'll probably need to redirect that to something like Debugee.allThreads() so it can also include these extra registered threads. Since many tests use their own EventHandler instance (not the one created by TestDebuggerType1), it may be easiest to just retrofit the breakpoint handler into the EventHandler.run() method rather than using the setCommunicationBreakpoint() approach, which adds a listener to the EventHandler.

            cjplummer Chris Plummer
            cjplummer Chris Plummer
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: