From 1ce978a041d644ce99ee7c70796e4c62fa799680 Mon Sep 17 00:00:00 2001
From: Jesse Glick <jglick@cloudbees.com>
Date: Tue, 16 May 2023 03:02:05 -0400
Subject: [PATCH] Refinements to `AssociatedConverterImpl.cache` (#7976)

* Refinements to `AssociatedConverterImpl.cache`

* Maintain a separate static cache just for the class loading
---
 core/src/main/java/hudson/util/XStream2.java | 47 ++++++++++++++------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java
index 355c500154..5c6ebc5667 100644
--- a/core/src/main/java/hudson/util/XStream2.java
+++ b/core/src/main/java/hudson/util/XStream2.java
@@ -469,8 +469,18 @@ public class XStream2 extends XStream {
      */
     private static final class AssociatedConverterImpl implements Converter {
         private final XStream xstream;
-        private final ConcurrentHashMap<Class<?>, Converter> cache =
-                new ConcurrentHashMap<>();
+        private static final ClassValue<Class<? extends ConverterMatcher>> classCache = new ClassValue<Class<? extends ConverterMatcher>>() {
+            @Override
+            protected Class<? extends ConverterMatcher> computeValue(Class<?> type) {
+                return computeConverterClass(type);
+            }
+        };
+        private final ClassValue<Converter> cache = new ClassValue<Converter>() {
+            @Override
+            protected Converter computeValue(Class<?> type) {
+                return computeConverter(type);
+            }
+        };
 
         private AssociatedConverterImpl(XStream xstream) {
             this.xstream = xstream;
@@ -481,17 +491,33 @@ public class XStream2 extends XStream {
             if (t == null) {
                 return null;
             }
+            return cache.get(t);
+        }
 
-            Converter result = cache.get(t);
-            if (result != null)
-                // ConcurrentHashMap does not allow null, so use this object to represent null
-                return result == this ? null : result;
+        @CheckForNull
+        private static Class<? extends ConverterMatcher> computeConverterClass(@NonNull Class<?> t) {
             try {
                 final ClassLoader classLoader = t.getClassLoader();
                 if (classLoader == null) {
                     return null;
                 }
-                Class<?> cl = classLoader.loadClass(t.getName() + "$ConverterImpl");
+                String name = t.getName() + "$ConverterImpl";
+                if (classLoader.getResource(name.replace('.', '/') + ".class") == null) {
+                    return null;
+                }
+                return classLoader.loadClass(name).asSubclass(ConverterMatcher.class);
+            } catch (ClassNotFoundException e) {
+                return null;
+            }
+        }
+
+        @CheckForNull
+        private Converter computeConverter(@NonNull Class<?> t) {
+            Class<? extends ConverterMatcher> cl = classCache.get(t);
+            if (cl == null) {
+                return null;
+            }
+            try {
                 Constructor<?> c = cl.getConstructors()[0];
 
                 Class<?>[] p = c.getParameterTypes();
@@ -506,14 +532,9 @@ public class XStream2 extends XStream {
 
                 }
                 ConverterMatcher cm = (ConverterMatcher) c.newInstance(args);
-                result = cm instanceof SingleValueConverter
+                return cm instanceof SingleValueConverter
                         ? new SingleValueConverterWrapper((SingleValueConverter) cm)
                         : (Converter) cm;
-                cache.put(t, result);
-                return result;
-            } catch (ClassNotFoundException e) {
-                cache.put(t, this);  // See above.. this object in cache represents null
-                return null;
             } catch (IllegalAccessException e) {
                 IllegalAccessError x = new IllegalAccessError();
                 x.initCause(e);