On Wed, 26 Feb 2025 19:46:41 GMT, Alan Bateman <al...@openjdk.org> wrote:

> Data structures that are accessed by both virtual threads and their carriers 
> require the virtual thread to pin the continuation to avoid potential 
> deadlock. A deadlock can arise when a virtual thread is preempted, is 
> selected and scheduled to be the next owner of the lock/resource, but can't 
> execute because all carriers are blocking on the same lock/resource. There 
> are a small number of places that need to pin. One that was missed is the the 
> notification to the thread container when threads are started or terminate. 
> This is not currently an issue at this time but it is a potential hazard for 
> ongoing and future work that will add further scheduling points to the code.  
> Continuation.pin/unpin have intrinsics since JDK-8338745, and Continuation is 
> initialized early in startup. Finally, the changes have been in the loom repo 
> for several months with no issues.
> 
> Testing tier1-5, quick statup/footprint, noreg-hard

Nice work, Alan

src/java.base/share/classes/jdk/internal/vm/ContinuationSupport.java line 57:

> 55:      */
> 56:     public static void pinIfSupported() {
> 57:         if (ContinuationSupport.isSupported()) {

Suggestion:

        if (isSupported()) {

src/java.base/share/classes/jdk/internal/vm/ContinuationSupport.java line 66:

> 64:      */
> 65:     public static void unpinIfSupported() {
> 66:         if (ContinuationSupport.isSupported()) {

Suggestion:

        if (isSupported()) {

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

Changes requested by vklang (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23810#pullrequestreview-2829366716
PR Review Comment: https://git.openjdk.org/jdk/pull/23810#discussion_r2082318599
PR Review Comment: https://git.openjdk.org/jdk/pull/23810#discussion_r2082318783

Reply via email to