0001-Use-computeIfAbsent-for-safe-ClassValue-calc.patch

From 0083ab83b69cc57c931ee7e9c261a3c3ce5d4aaa Mon Sep 17 00:00:00 2001
From: Charles Oliver Nutter <headius@headius.com>
Date: Fri, 19 Mar 2021 10:14:04 -0500
Subject: [PATCH] Use computeIfAbsent for safe ClassValue calc

---
 .../jruby/javasupport/JavaSupportImpl.java    | 29 +++++--------------
 .../jruby/util/collections/ClassValue.java    | 14 +++++----
 .../collections/ClassValueCalculator.java     |  8 -----
 .../util/collections/Java7ClassValue.java     |  5 ++--
 .../util/collections/MapBasedClassValue.java  | 18 ++----------
 5 files changed, 21 insertions(+), 53 deletions(-)
 delete mode 100644 core/src/main/java/org/jruby/util/collections/ClassValueCalculator.java

diff --git a/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java b/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java
index 9a892853cc..bf10f4f50c 100644
--- a/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java
+++ b/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java
@@ -58,7 +58,6 @@ import org.jruby.javasupport.proxy.JavaProxyClass;
 import org.jruby.javasupport.util.ObjectProxyCache;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.util.WeakIdentityHashMap;
-import org.jruby.util.collections.ClassValueCalculator;
 
 public class JavaSupportImpl extends JavaSupport {
     private final Ruby runtime;
@@ -109,35 +108,21 @@ public class JavaSupportImpl extends JavaSupport {
     public JavaSupportImpl(final Ruby runtime) {
         this.runtime = runtime;
 
-        this.javaClassCache = ClassValue.newInstance(new ClassValueCalculator<JavaClass>() {
-            @Override
-            public JavaClass computeValue(Class<?> klass) {
-                return new JavaClass(runtime, getJavaClassClass(), klass);
-            }
-        });
+        this.javaClassCache = ClassValue.newInstance(klass -> new JavaClass(runtime, getJavaClassClass(), klass));
 
-        this.proxyClassCache = ClassValue.newInstance(new ClassValueCalculator<RubyModule>() {
+        this.proxyClassCache = ClassValue.newInstance(klass -> {
             /**
              * Because of the complexity of processing a given class and all its dependencies,
              * we opt to synchronize this logic. Creation of all proxies goes through here,
              * allowing us to skip some threading work downstream.
              */
-            @Override
-            public synchronized RubyModule computeValue(Class<?> klass) {
-                RubyModule proxyKlass = Java.createProxyClassForClass(runtime, klass);
-                JavaExtensions.define(runtime, klass, proxyKlass); // (lazy) load extensions
-                return proxyKlass;
-            }
+            RubyModule proxyKlass = Java.createProxyClassForClass(runtime, klass);
+            JavaExtensions.define(runtime, klass, proxyKlass); // (lazy) load extensions
+            return proxyKlass;
         });
 
-        this.staticAssignedNames = ClassValue.newInstance(new ClassValueCalculator<Map<String, AssignedName>>() {
-            @Override
-            public Map<String, AssignedName> computeValue(Class<?> cls) { return new HashMap<>(); }
-        });
-        this.instanceAssignedNames = ClassValue.newInstance(new ClassValueCalculator<Map<String, AssignedName>>() {
-            @Override
-            public Map<String, AssignedName> computeValue(Class<?> cls) { return new HashMap<>(); }
-        });
+        this.staticAssignedNames = ClassValue.newInstance(cls -> new HashMap<>());
+        this.instanceAssignedNames = ClassValue.newInstance(cls -> new HashMap<>());
 
         // Proxy creation is synchronized (see above) so a HashMap is fine for recursion detection.
         this.unfinishedProxies = new ConcurrentHashMap<>(8, 0.75f, 1);
diff --git a/core/src/main/java/org/jruby/util/collections/ClassValue.java b/core/src/main/java/org/jruby/util/collections/ClassValue.java
index f271e7db74..b0fc136e42 100644
--- a/core/src/main/java/org/jruby/util/collections/ClassValue.java
+++ b/core/src/main/java/org/jruby/util/collections/ClassValue.java
@@ -2,6 +2,8 @@ package org.jruby.util.collections;
 
 import org.jruby.util.cli.Options;
 
+import java.util.function.Function;
+
 /**
  * Represents a cache or other mechanism for getting the Ruby-level proxy classes
  * for a given Java class.
@@ -9,21 +11,21 @@ import org.jruby.util.cli.Options;
 @SuppressWarnings("unchecked")
 public abstract class ClassValue<T> {
 
-    public ClassValue(ClassValueCalculator<T> calculator) {
+    public ClassValue(Function<Class<?>, T> calculator) {
         this.calculator = calculator;
     }
 
     public abstract T get(Class<?> cls);
 
-    protected final ClassValueCalculator<T> calculator;
+    protected final Function<Class<?>, T> calculator;
 
-    public static <T> ClassValue<T> newInstance(ClassValueCalculator<T> calculator) {
+    public static <T> ClassValue<T> newInstance(Function<Class<?>, T> calculator) {
         if ( CLASS_VALUE ) return newJava7Instance(calculator);
-        return new MapBasedClassValue<>(calculator);
+        return new MapBasedClassValue<T>(calculator);
     }
 
-    private static <T> ClassValue<T> newJava7Instance(ClassValueCalculator<T> calculator) {
-        return new Java7ClassValue<>(calculator);
+    private static <T> ClassValue<T> newJava7Instance(Function<Class<?>, T> calculator) {
+        return new Java7ClassValue<T>(calculator);
     }
 
     private static final boolean CLASS_VALUE = Options.INVOKEDYNAMIC_CLASS_VALUES.load();
diff --git a/core/src/main/java/org/jruby/util/collections/ClassValueCalculator.java b/core/src/main/java/org/jruby/util/collections/ClassValueCalculator.java
deleted file mode 100644
index 1a629f27da..0000000000
--- a/core/src/main/java/org/jruby/util/collections/ClassValueCalculator.java
+++ /dev/null
@@ -1,8 +0,0 @@
-package org.jruby.util.collections;
-
-/**
- * Calculate a value based on an incoming class. Used by ClassValue.
- */
-public interface ClassValueCalculator<T> {
-    public T computeValue(Class<?> cls);
-}
diff --git a/core/src/main/java/org/jruby/util/collections/Java7ClassValue.java b/core/src/main/java/org/jruby/util/collections/Java7ClassValue.java
index 466be52d82..7a595afc6f 100644
--- a/core/src/main/java/org/jruby/util/collections/Java7ClassValue.java
+++ b/core/src/main/java/org/jruby/util/collections/Java7ClassValue.java
@@ -3,13 +3,14 @@ package org.jruby.util.collections;
 import java.lang.ref.WeakReference;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.function.Function;
 
 /**
  * A proxy cache that uses Java 7's ClassValue.
  */
 final class Java7ClassValue<T> extends ClassValue<T> {
 
-    public Java7ClassValue(ClassValueCalculator<T> calculator) {
+    public Java7ClassValue(Function<Class<?>, T> calculator) {
         super(calculator);
     }
 
@@ -33,7 +34,7 @@ final class Java7ClassValue<T> extends ClassValue<T> {
 
         @Override
         protected WeakReference<T> computeValue(Class<?> type) {
-            T value = calculator.computeValue(type);
+            T value = calculator.apply(type);
             computedValues.add(value);
             return new WeakReference(value);
         }
diff --git a/core/src/main/java/org/jruby/util/collections/MapBasedClassValue.java b/core/src/main/java/org/jruby/util/collections/MapBasedClassValue.java
index f1fec62265..6cfbe51658 100644
--- a/core/src/main/java/org/jruby/util/collections/MapBasedClassValue.java
+++ b/core/src/main/java/org/jruby/util/collections/MapBasedClassValue.java
@@ -1,32 +1,20 @@
 package org.jruby.util.collections;
 
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 /**
  * A simple Map-based cache of proxies.
  */
 public final class MapBasedClassValue<T> extends ClassValue<T> {
     
-    public MapBasedClassValue(ClassValueCalculator<T> calculator) {
+    public MapBasedClassValue(Function<Class<?>, T> calculator) {
         super(calculator);
     }
 
     @Override
     public T get(Class<?> cls) {
-        T obj = cache.get(cls);
-
-        if (obj != null) return obj;
-
-        synchronized (this) {
-            obj = cache.get(cls);
-
-            if (obj != null) return obj;
-
-            obj = calculator.computeValue(cls);
-            cache.put(cls, obj);
-        }
-
-        return obj;
+        return cache.computeIfAbsent(cls, calculator);
     }
 
     // There's not a compelling reason to keep JavaClass instances in a weak map
-- 
2.23.0