On Fri, 22 Sep 2023 21:40:14 GMT, Leonid Mesnik <[email protected]> wrote:
>> The code related to virtual threads support is moved into a separate class
>> DebugeeWrapper.
>>
>> The code of method main() remains the same. I don't mix code change with
>> moving code between files.
>>
>> Tesed by running
>> make run-test JTREG_RETAIN=all TEST=com/sun/jdi
>> make run-test JTREG_RETAIN=all JTREG_TEST_THREAD_FACTORY=Virtual
>> TEST=com/sun/jdi
>> locally and tier1 and hs-tier5 in Mach5 CI
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fixed comment
test/jdk/com/sun/jdi/TestScaffold.java line 555:
> 553: if (mainWrapper != null &&
> !argInfo.targetAppCommandLine.isEmpty()) {
> 554: argInfo.targetVMArgs += "-D" + DebuggeeWrapper.PROPERTY_NAME
> + "=" + mainWrapper;
> 555: argInfo.targetAppCommandLine =
> DebuggeeWrapper.class.getName() + ' '
I know this is a pre-existing issue, but it just seems strange that we have to
both set the main.wrapper property to "Virtual" and also pass "Virtual" as the
first argument to DebuggeeWrapper.main(). Could we leave it up to
DebuggeeWrapper.main() to set main.wrapper when the first arg is "Virtual"?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15874#discussion_r1334837498
