On Tue, 5 Dec 2023 09:51:50 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> When a virtual thread continues after Thread.yield it currently consumes 
>> thread's parking permit. This is an oversight, the parking permit should 
>> only be consumed when continuing after park.
>> 
>> The changes are straight-forward. The internal "RUNNABLE" state is replaced 
>> with UNPARKED and YIELDED state, effectively encoding the previous state. So 
>> for the most part, it's just replacing the usages of RUNNABLE. The 
>> additional states require refactoring tryGetStackTrace, this is the method 
>> that is used for Thread::getStackTrace when the virtual thread is unmounted. 
>> It is also changed to not not swallow the REE if the reesubmit fails 
>> (tryStackTrace has to resubmit as the task for the thread may run, or the 
>> thread unparked, while "suspended" and sampling its stack trace). The 
>> changes are a subset of larger changes in the loom repo, future PRs will 
>> propose to bring in other changes to get main line up to date.
>> 
>> For testing the existing ThreadAPI has new test cases.
>> 
>> Testing: test1-5. This includes the JVMTI tests as it maps the thread states 
>> to JVMTI thread states.
>
> Alan Bateman has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - Fix comment, remove space between case labels and colon
>  - Merge
>  - Leave onPinned to another PR
>  - Initial commit

I find the switch from `RUNNABLE` to `UNPARKED` somewhat unnecessary. I don't 
see any problem with `RUNNABLE` that needed fixing. I found the distinction and 
transition between `RUNNABLE` and `RUNNING` to be very clear. ??

-------------

PR Review: https://git.openjdk.org/jdk/pull/16953#pullrequestreview-1766538038

Reply via email to