On Mon, 12 Jun 2023 14:09:59 GMT, Andrew Haley <a...@openjdk.org> wrote:
>> Move `runWith()` from `VirtualThread` to `BaseVirtualThread`.
>>
>> `BoundVirtualThread` does not use `runWith()` to run its task, so when a VM
>> error occurs it can not recover scoped values.
>>
>> Moving `runWith()` into the common subclass of both `VirtualThread` and
>> `BoundVirtualThread` fixes the problem.
>
> Andrew Haley has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Whitespace
> - Use RunWith in base Thread class & enable testing without VM Continuations.
src/java.base/share/classes/java/lang/ThreadBuilders.java line 436:
> 434: if (Thread.currentThread() == this && !runInvoked) {
> 435: runInvoked = true;
> 436: Object bindings = scopedValueBindings();
Would it be possible to make this Thread.scopedValueBindings() to make it more
obvious that it's a static method. Maybe the same in VirtualThread.run.
src/java.base/share/classes/java/lang/VirtualThread.java line 27:
> 25: package java.lang;
> 26:
> 27: import java.lang.ref.Reference;
I think the import of ForceInline can be removed too.
test/jdk/ProblemList-Virtual.txt line 48:
> 46:
> 47: java/lang/ScopedValue/StressStackOverflow.java 8309646 generic-all
> 48:
That's the issue for the JTREG_TEST_THREAD_FACTORY=Virtual runs. I think we
need to leave it excluded until that issue is diagnosed/fixed.
However, in jdk/test/ProblemList.txt, it is excluded on linux-s390x as that
platform doesn't seem to have, or use, VM continuations and so is using the
fallback/alternative implementation. I think this entry can be removed now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14399#discussion_r1226766408
PR Review Comment: https://git.openjdk.org/jdk/pull/14399#discussion_r1226767359
PR Review Comment: https://git.openjdk.org/jdk/pull/14399#discussion_r1226764146
