RFR: 8343132: Remove temporary transitions from Virtual thread implementation [v2]

[Alan Bateman]

On Thu, 31 Oct 2024 04:43:21 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 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 typo in comment
>>  - Merge branch 'master' into JDK-8343132
>>  - Merge branch 'master' into JDK-8343132
>>  - Initial commit
>
> src/java.base/share/classes/java/lang/VirtualThread.java line 537:
> 
>> 535:                 assert parkTimeout > 0;
>> 536:                 timeoutTask = schedule(this::unpark, parkTimeout, NANOSECONDS);
>> 537:                 setState(newState = TIMED_PARKED);
> 
> Just to be clear here, if the timeout expires before we can call `setState`, the unpark is basically a no-op, and we will see that we have been unparked at line 541 and set the state correctly to UNPARKED.

Yes, and same thing is unparked by some other thread while the target thread is parking. We have several tests that bash on this.

> test/jdk/java/lang/Thread/virtual/ParkWithFixedThreadPool.java line 93:
> 
>> 91:             } finally {
>> 92:                 // ExecutorService::execute may consume parking permit
>> 93:                 LockSupport.unpark(Thread.currentThread());
> 
> This seems a bit odd - why would the current thread need to unpark itself? Why should it have a park permit available here?

In this test, Scheduler.execute method will consume the current thread's parking permit when there is contention on the queue. In a well behaved system, all usages of park will first test some condition before parking. This test doesn't do this, hence it created the scenario where parking after unparking might hang. Previous discussion in [loom/pull/59](https://github.com/openjdk/loom/pull/59). There is no support exposed for doing custom schedulers at this time but this is the type of thing that comes up so we kept the test.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21735#discussion_r1823905112
PR Review Comment: https://git.openjdk.org/jdk/pull/21735#discussion_r1823903551