On Wed, 29 Mar 2023 21:43:38 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix ThreadSleepEvent again > > src/java.base/share/classes/java/lang/Thread.java line 1546: > >> 1544: // bind thread to container >> 1545: if (this.container != null) >> 1546: throw new IllegalThreadStateException(); > > This check is not replicated in `VirtualThread::start`, i think the CAS > protects against that. Maybe assert instead in the virtual thread > implementation, thereby the comment in `setThreadContainer` can be changed to > something like "`this.container` checked/asserted to be != null before call > to Virtual/Thread::start(ThreadContainer)" ? Yes, they are different. If adding platform thread to a container fails with OOME then it does so when its threadStatus is 0. This check just previous another attempt to start it. Virtual threads work differently but I can add an assert to make this clearer. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13203#discussion_r1153726790
