8357637: Native resources cached in thread locals not released when FJP common pool threads clears thread locals

Reviewed-by: vklang

Author: Alan Bateman

src/java.base/share/classes/java/lang/InheritableThreadLocal.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -80,7 +80,7 @@ protected T childValue(T parentValue) {
      */
     @Override
     ThreadLocalMap getMap(Thread t) {
-        return t.inheritableThreadLocals;
+        return t.inheritableThreadLocals();
     }
 
     /**
@@ -91,6 +91,6 @@ ThreadLocalMap getMap(Thread t) {
      */
     @Override
     void createMap(Thread t, T firstValue) {
-        t.inheritableThreadLocals = new ThreadLocalMap(this, firstValue);
+        t.setInheritableThreadLocals(new ThreadLocalMap(this, firstValue));
     }
 }

src/java.base/share/classes/java/lang/System.java

@@ -2254,10 +2254,6 @@ public void removeCarrierThreadLocal(CarrierThreadLocal<?> local) {
                 ((ThreadLocal<?>)local).removeCarrierThreadLocal();
             }
 
-            public boolean isCarrierThreadLocalPresent(CarrierThreadLocal<?> local) {
-                return ((ThreadLocal<?>)local).isCarrierThreadLocalPresent();
-            }
-
             public Object[] scopedValueCache() {
                 return Thread.scopedValueCache();
             }

src/java.base/share/classes/java/lang/Thread.java

@@ -235,7 +235,7 @@ public class Thread implements Runnable {
     private volatile ClassLoader contextClassLoader;
 
     // Additional fields for platform threads.
-    // All fields, except task, are accessed directly by the VM.
+    // All fields, except task and terminatingThreadLocals, are accessed directly by the VM.
     private static class FieldHolder {
         final ThreadGroup group;
         final Runnable task;
@@ -244,6 +244,9 @@ private static class FieldHolder {
         volatile boolean daemon;
         volatile int threadStatus;
 
+        // This map is maintained by the ThreadLocal class
+        ThreadLocal.ThreadLocalMap terminatingThreadLocals;
+
         FieldHolder(ThreadGroup group,
                     Runnable task,
                     long stackSize,
@@ -259,17 +262,41 @@ private static class FieldHolder {
     }
     private final FieldHolder holder;
 
+    ThreadLocal.ThreadLocalMap terminatingThreadLocals() {
+        return holder.terminatingThreadLocals;
+    }
+
+    void setTerminatingThreadLocals(ThreadLocal.ThreadLocalMap map) {
+        holder.terminatingThreadLocals = map;
+    }
+
     /*
      * ThreadLocal values pertaining to this thread. This map is maintained
      * by the ThreadLocal class.
      */
-    ThreadLocal.ThreadLocalMap threadLocals;
+    private ThreadLocal.ThreadLocalMap threadLocals;
+
+    ThreadLocal.ThreadLocalMap threadLocals() {
+        return threadLocals;
+    }
+
+    void setThreadLocals(ThreadLocal.ThreadLocalMap map) {
+        threadLocals = map;
+    }
 
     /*
      * InheritableThreadLocal values pertaining to this thread. This map is
      * maintained by the InheritableThreadLocal class.
      */
-    ThreadLocal.ThreadLocalMap inheritableThreadLocals;
+    private ThreadLocal.ThreadLocalMap inheritableThreadLocals;
+
+    ThreadLocal.ThreadLocalMap inheritableThreadLocals() {
+        return inheritableThreadLocals;
+    }
+
+    void setInheritableThreadLocals(ThreadLocal.ThreadLocalMap map) {
+        inheritableThreadLocals = map;
+    }
 
     /*
      * Scoped value bindings are maintained by the ScopedValue class.
@@ -1492,7 +1519,7 @@ private void exit() {
         }
 
         try {
-            if (threadLocals != null && TerminatingThreadLocal.REGISTRY.isPresent()) {
+            if (terminatingThreadLocals() != null) {
                 TerminatingThreadLocal.threadTerminated();
             }
         } finally {

src/java.base/share/classes/java/lang/ThreadLocal.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -193,27 +193,6 @@ private T get(Thread t) {
         return setInitialValue(t);
     }
 
-    /**
-     * Returns {@code true} if there is a value in the current carrier thread's copy of
-     * this thread-local variable, even if that values is {@code null}.
-     *
-     * @return {@code true} if current carrier thread has associated value in this
-     *         thread-local variable; {@code false} if not
-     */
-    boolean isCarrierThreadLocalPresent() {
-        assert this instanceof CarrierThreadLocal<T>;
-        return isPresent(Thread.currentCarrierThread());
-    }
-
-    private boolean isPresent(Thread t) {
-        ThreadLocalMap map = getMap(t);
-        if (map != null) {
-            return map.getEntry(this) != null;
-        } else {
-            return false;
-        }
-    }
-
     /**
      * Variant of set() to establish initialValue. Used instead
      * of set() in case user has overridden the set() method.
@@ -302,7 +281,11 @@ private void remove(Thread t) {
      * @return the map
      */
     ThreadLocalMap getMap(Thread t) {
-        return t.threadLocals;
+        if (this instanceof TerminatingThreadLocal<T>) {
+            return t.terminatingThreadLocals();
+        } else {
+            return t.threadLocals();
+        }
     }
 
     /**
@@ -313,7 +296,12 @@ ThreadLocalMap getMap(Thread t) {
      * @param firstValue value for the initial entry of the map
      */
     void createMap(Thread t, T firstValue) {
-        t.threadLocals = new ThreadLocalMap(this, firstValue);
+        var map = new ThreadLocalMap(this, firstValue);
+        if (this instanceof TerminatingThreadLocal<T>) {
+            t.setTerminatingThreadLocals(map);
+        } else {
+            t.setThreadLocals(map);
+        }
     }
 
     /**

src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java

@@ -546,12 +546,6 @@ public interface JavaLangAccess {
      */
     void removeCarrierThreadLocal(CarrierThreadLocal<?> local);
 
-    /**
-     * Returns {@code true} if there is a value in the current carrier thread's copy of
-     * thread-local, even if that values is {@code null}.
-     */
-    boolean isCarrierThreadLocalPresent(CarrierThreadLocal<?> local);
-
     /**
      * Returns the current thread's scoped values cache
      */

src/java.base/share/classes/jdk/internal/misc/CarrierThreadLocal.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -49,9 +49,5 @@ public void remove() {
         JLA.removeCarrierThreadLocal(this);
     }
 
-    public boolean isPresent() {
-        return JLA.isCarrierThreadLocalPresent(this);
-    }
-
     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();
 }

src/java.base/share/classes/jdk/internal/misc/TerminatingThreadLocal.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,10 +29,9 @@
 import java.util.IdentityHashMap;
 
 /**
- * A per-carrier-thread-local variable that is notified when a thread terminates and
- * it has been initialized in the terminating carrier thread or a virtual thread
- * that had the terminating carrier thread as its carrier thread (even if it was
- * initialized with a null value).
+ * A platform thread-local variable that is notified when a platform thread terminates,
+ * and it has been initialized in the terminating thread (or a mounted virtual thread in
+ * the case of a carrier thread), and even if it was initialized with a null value.
  */
 public class TerminatingThreadLocal<T> extends CarrierThreadLocal<T> {
 
@@ -44,8 +43,8 @@ public void set(T value) {
 
     @Override
     public void remove() {
-        super.remove();
         unregister(this);
+        super.remove();
     }
 
     /**
@@ -80,7 +79,9 @@ public static void threadTerminated() {
      * @param tl the ThreadLocal to register
      */
     public static void register(TerminatingThreadLocal<?> tl) {
-        REGISTRY.get().add(tl);
+        if (tl != REGISTRY) {
+            REGISTRY.get().add(tl);
+        }
     }
 
     /**
@@ -93,11 +94,11 @@ private static void unregister(TerminatingThreadLocal<?> tl) {
     }
 
     /**
-     * a per-carrier-thread registry of TerminatingThreadLocal(s) that have been registered
-     * but later not unregistered in a particular carrier-thread.
+     * A per-platform-thread registry of TerminatingThreadLocal(s). The registry is
+     * itself a TerminatingThreadLocal to keep it reachable until the thread terminates.
      */
-    public static final CarrierThreadLocal<Collection<TerminatingThreadLocal<?>>> REGISTRY =
-        new CarrierThreadLocal<>() {
+    public static final TerminatingThreadLocal<Collection<TerminatingThreadLocal<?>>> REGISTRY =
+        new TerminatingThreadLocal<>() {
             @Override
             protected Collection<TerminatingThreadLocal<?>> initialValue() {
                 return Collections.newSetFromMap(new IdentityHashMap<>(4));

src/java.base/share/classes/sun/nio/ch/IOVecWrapper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -79,6 +79,7 @@ protected IOVecWrapper[] initialValue() {
         protected void threadTerminated(IOVecWrapper[] cache) {
             IOVecWrapper wrapper = cache[0];
             if (wrapper != null) {
+                cache[0] = null;
                 wrapper.vecArray.free();
             }
         }

test/jdk/jdk/internal/misc/TerminatingThreadLocal/TestTerminatingThreadLocal.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,13 +24,15 @@
 import jdk.internal.misc.TerminatingThreadLocal;
 
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -41,7 +43,7 @@
 
 /*
  * @test
- * @bug 8202788 8291897
+ * @bug 8202788 8291897 8357637
  * @summary TerminatingThreadLocal unit test
  * @modules java.base/java.lang:+open java.base/jdk.internal.misc
  * @requires vm.continuations
@@ -154,6 +156,53 @@ protected T initialValue() {
         assertEquals(terminatedValues, expectedTerminatedValues);
     }
 
+    /**
+     * Test TerminatingThreadLocal when thread locals are "cleared" by null'ing the
+     * threadLocal field of the current Thread.
+     */
+    @Test
+    public void testClearingThreadLocals() throws Throwable {
+        var terminatedValues = new CopyOnWriteArrayList<Object>();
+
+        var tl = new ThreadLocal<String>();
+        var ttl = new TerminatingThreadLocal<String>() {
+            @Override
+            protected void threadTerminated(String value) {
+                terminatedValues.add(value);
+            }
+        };
+        var throwableRef = new AtomicReference<Throwable>();
+
+        String tlValue = "abc";
+        String ttlValue = "xyz";
+
+        Thread thread = Thread.ofPlatform().start(() -> {
+            try {
+                tl.set(tlValue);
+                ttl.set(ttlValue);
+
+                assertEquals(tl.get(), tlValue);
+                assertEquals(ttl.get(), ttlValue);
+
+                // set Thread.threadLocals to null
+                Field f = Thread.class.getDeclaredField("threadLocals");
+                f.setAccessible(true);
+                f.set(Thread.currentThread(), null);
+
+                assertNull(tl.get());
+                assertEquals(ttl.get(), ttlValue);
+            } catch (Throwable t) {
+                throwableRef.set(t);
+            }
+        });
+        thread.join();
+        if (throwableRef.get() instanceof Throwable t) {
+            throw t;
+        }
+
+        assertEquals(terminatedValues, List.of(ttlValue));
+    }
+
     /**
      * Returns a builder to create virtual threads that use the given scheduler.
      */