8136700: Make sure Context.anonymousHostClasses doesn't grow unbounded
authorattila
Fri, 25 Sep 2015 15:57:57 +0200
changeset 32786 dedf2b6ea495
parent 32785 3ed9323fb0cc
child 32787 c49a2638ad02
8136700: Make sure Context.anonymousHostClasses doesn't grow unbounded Reviewed-by: hannesw, sundar
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java	Fri Sep 25 12:46:53 2015 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java	Fri Sep 25 15:57:57 2015 +0200
@@ -64,7 +64,6 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.atomic.LongAdder;
@@ -301,7 +300,47 @@
         }
     }
 
-    private final Map<CodeSource, Reference<Class<?>>> anonymousHostClasses = new ConcurrentHashMap<>();
+    private final Map<CodeSource, HostClassReference> anonymousHostClasses = new HashMap<>();
+    private final ReferenceQueue<Class<?>> anonymousHostClassesRefQueue = new ReferenceQueue<>();
+
+    private static class HostClassReference extends WeakReference<Class<?>> {
+        final CodeSource codeSource;
+
+        HostClassReference(final CodeSource codeSource, final Class<?> clazz, final ReferenceQueue<Class<?>> refQueue) {
+            super(clazz, refQueue);
+            this.codeSource = codeSource;
+        }
+    }
+
+    private synchronized Class<?> getAnonymousHostClass(final CodeSource codeSource) {
+        // Remove cleared entries
+        for(;;) {
+            final HostClassReference clearedRef = (HostClassReference)anonymousHostClassesRefQueue.poll();
+            if (clearedRef == null) {
+                break;
+            }
+            anonymousHostClasses.remove(clearedRef.codeSource, clearedRef);
+        }
+
+        // Try to find an existing host class
+        final Reference<Class<?>> ref = anonymousHostClasses.get(codeSource);
+        if (ref != null) {
+            final Class<?> existingHostClass = ref.get();
+            if (existingHostClass != null) {
+                return existingHostClass;
+            }
+        }
+
+        // Define a new host class if existing is not found
+        final Class<?> newHostClass = createNewLoader().installClass(
+                // NOTE: we're defining these constants in AnonymousContextCodeInstaller so they are not
+                // initialized if we don't use AnonymousContextCodeInstaller. As this method is only ever
+                // invoked from AnonymousContextCodeInstaller, this is okay.
+                AnonymousContextCodeInstaller.ANONYMOUS_HOST_CLASS_NAME,
+                AnonymousContextCodeInstaller.ANONYMOUS_HOST_CLASS_BYTES, codeSource);
+        anonymousHostClasses.put(codeSource, new HostClassReference(codeSource, newHostClass, anonymousHostClassesRefQueue));
+        return newHostClass;
+    }
 
     private static final class AnonymousContextCodeInstaller extends ContextCodeInstaller {
         private static final Unsafe UNSAFE = getUnsafe();
@@ -310,9 +349,9 @@
 
         private final Class<?> hostClass;
 
-        private AnonymousContextCodeInstaller(final Context context, final CodeSource codeSource) {
+        private AnonymousContextCodeInstaller(final Context context, final CodeSource codeSource, final Class<?> hostClass) {
             super(context, codeSource);
-            hostClass = getAnonymousHostClass();
+            this.hostClass = hostClass;
         }
 
         @Override
@@ -335,19 +374,6 @@
             return new NamedContextCodeInstaller(context, codeSource, context.createNewLoader());
         }
 
-        private Class<?> getAnonymousHostClass() {
-            final Reference<Class<?>> ref = context.anonymousHostClasses.get(codeSource);
-            if (ref != null) {
-                final Class<?> existingHostClass = ref.get();
-                if (existingHostClass != null) {
-                    return existingHostClass;
-                }
-            }
-            final Class<?> newHostClass = context.createNewLoader().installClass(ANONYMOUS_HOST_CLASS_NAME, ANONYMOUS_HOST_CLASS_BYTES, codeSource);
-            context.anonymousHostClasses.put(codeSource, new WeakReference<>(newHostClass));
-            return newHostClass;
-        }
-
         private static final byte[] getAnonymousHostClassBytes() {
             final ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS);
             cw.visit(V1_7, Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT, ANONYMOUS_HOST_CLASS_NAME.replace('.', '/'), null, "java/lang/Object", null);
@@ -1414,7 +1440,7 @@
             final ScriptLoader loader = env._loader_per_compile ? createNewLoader() : scriptLoader;
             installer = new NamedContextCodeInstaller(this, cs, loader);
         } else {
-            installer = new AnonymousContextCodeInstaller(this, cs);
+            installer = new AnonymousContextCodeInstaller(this, cs, getAnonymousHostClass(cs));
         }
 
         if (storedScript == null) {