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