On Tue, 2 May 2023 21:16:20 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
> The debugger tests might start all debugee using virtual threads when
> property "main.wrapper" is set.
> However, the new mode JTREG_TEST_THREAD_FACTORY do the same thing. The only
> difference is that it use predefined problemlist names and doesn't set
> property "main.wrapper" as a part of jtreg properties. So nsk wrapper should
> set it additionally.
I think MainWrapper.java could use some documentation on why/how/when it used.
This is something that should have been done before first committing it when
loom changes were integrated.
Also, I see what looks like a bug w.r.t. this code in Launcher.java:
if (System.getProperty("main.wrapper") != null) {
cmdline = MainWrapper.class.getName() + " " +
System.getProperty("main.wrapper") + " " + cmdline;
}
It gets the main.wrapper property in order to pass it to MainWrapper.main(),
which is in charge of setting the properly, so how could it ever already be set
when this Launcher.java code is executed? Same thing in Binder.java and
DebugeeBinder.java.
test/hotspot/jtreg/vmTestbase/nsk/share/MainWrapper.java line 50:
> 48:
> 49: // Some tests use this property to understand if virtual threads
> are used
> 50: System.setProperty("main.wrapper", "Virtual");
Shouldn't this be:
`System.setProperty("main.wrapper", wrapperName);`
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13763#issuecomment-1532316167
PR Review Comment: https://git.openjdk.org/jdk/pull/13763#discussion_r1183151407
