Fixes linkedin#124: Include interfaces in the proxy class cache key

mahuna13 · Nov 15, 2018 · d0d5b5d · d0d5b5d
1 parent 32b463e
commit d0d5b5d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 13 deletions.
diff --git a/dexmaker-tests/src/androidTest/java/com/android/dx/stock/ProxyBuilderTest.java b/dexmaker-tests/src/androidTest/java/com/android/dx/stock/ProxyBuilderTest.java
@@ -33,8 +33,11 @@
 import java.lang.reflect.UndeclaredThrowableException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Random;
+import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -1251,4 +1254,23 @@ public void testImplementingDifferentInterfacesWithSharedClassLoader() throws IO
                 .implementing(Callable.class).withSharedClassLoader().buildProxyClass();
         assertNotSame(c1, c2);
     }
+
+    @Test
+    public void testImplementingDifferentInterfacesWithDexMakerCaching() throws Exception {
+        proxyFor(SimpleClass.class).implementing(Runnable.class).buildProxyClass();
+        assertEquals(1, getGeneratedProxyClasses().size());
+        assertEquals(2, versionedDxDir.listFiles().length);
+
+        proxyFor(SimpleClass.class).implementing(Callable.class).buildProxyClass();
+        assertEquals(2, getGeneratedProxyClasses().size());
+        assertEquals(3, versionedDxDir.listFiles().length);
+
+        Map<?, ?> cacheCopy = new HashMap<>(getGeneratedProxyClasses());
+        Set<?> generatedFiles = new HashSet<>(Arrays.asList(versionedDxDir.listFiles()));
+
+        proxyFor(SimpleClass.class).implementing(Runnable.class).buildProxyClass();
+        proxyFor(SimpleClass.class).implementing(Callable.class).buildProxyClass();
+        assertEquals(cacheCopy, getGeneratedProxyClasses());
+        assertEquals(generatedFiles, new HashSet<>(Arrays.asList(versionedDxDir.listFiles())));
+    }
 }
diff --git a/dexmaker/src/main/java/com/android/dx/stock/ProxyBuilder.java b/dexmaker/src/main/java/com/android/dx/stock/ProxyBuilder.java
@@ -41,7 +41,6 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
 
 import static java.lang.reflect.Modifier.ABSTRACT;
 import static java.lang.reflect.Modifier.PRIVATE;
@@ -272,10 +271,11 @@ public Class<? extends T> buildProxyClass() throws IOException {
 
         // try the cache to see if we've generated this one before
         // we only populate the map with matching types
+        ProxiedClass<T> cacheKey =
+                new ProxiedClass<>(baseClass, interfaces, requestedClassloader, sharedClassLoader);
         @SuppressWarnings("unchecked")
-        Class<? extends T> proxyClass = (Class) generatedProxyClasses.get(
-                new ProxiedClass<>(baseClass, requestedClassloader, sharedClassLoader));
-        if (proxyClass != null && interfaces.equals(asSet(proxyClass.getInterfaces()))) {
+        Class<? extends T> proxyClass = (Class) generatedProxyClasses.get(cacheKey);
+        if (proxyClass != null) {
             return proxyClass; // cache hit!
         }
 
@@ -342,9 +342,7 @@ public int compare(Method method1, Method method2) {
             throw new AssertionError(e);
         }
         setMethodsStaticField(proxyClass, methodsToProxy);
-        generatedProxyClasses.put(new ProxiedClass<>(baseClass, requestedClassloader,
-                        sharedClassLoader),
-                proxyClass);
+        generatedProxyClasses.put(cacheKey, proxyClass);
         return proxyClass;
     }
 
@@ -849,10 +847,6 @@ private static void generateCodeForReturnStatement(Code code, Class methodReturn
         }
     }
 
-    private static <T> Set<T> asSet(T... array) {
-        return new CopyOnWriteArraySet<>(Arrays.asList(array));
-    }
-
     private static MethodId<?, ?> getUnboxMethodForPrimitive(Class<?> methodReturnType) {
         return PRIMITIVE_TO_UNBOX_METHOD.get(methodReturnType);
     }
@@ -950,6 +944,8 @@ public int hashCode() {
     private static class ProxiedClass<U> {
         final Class<U> clazz;
 
+        final Set<?> interfaces;
+
         /**
          * Class loader requested when the proxy class was generated. This might not be the
          * class loader of {@code clazz} as not all class loaders can be shared.
@@ -971,18 +967,21 @@ public boolean equals(Object other) {
 
             ProxiedClass<?> that = (ProxiedClass<?>) other;
             return clazz == that.clazz
+                    && interfaces.equals(that.interfaces)
                     && requestedClassloader == that.requestedClassloader
                     && sharedClassLoader == that.sharedClassLoader;
         }
 
         @Override
         public int hashCode() {
-            return clazz.hashCode() + requestedClassloader.hashCode() + (sharedClassLoader ? 1 : 0);
+            return clazz.hashCode() + interfaces.hashCode() + requestedClassloader.hashCode()
+                    + (sharedClassLoader ? 1 : 0); 
         }
 
-        private ProxiedClass(Class<U> clazz, ClassLoader requestedClassloader,
+        private ProxiedClass(Class<U> clazz, Set<?> interfaces, ClassLoader requestedClassloader,
                              boolean sharedClassLoader) {
             this.clazz = clazz;
+            this.interfaces = interfaces;
             this.requestedClassloader = requestedClassloader;
             this.sharedClassLoader = sharedClassLoader;
         }