Only create one UnsafeAllocator instance (#2196)

* Only create one UnsafeAllocator instance

* Move checkInstantiable to ConstructorConstructor

3 files changed, 28 insertions, 31 deletions

diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
index 68b2bd64..da330c30 100644
--- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
+++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
@@ -61,6 +61,25 @@ public final class ConstructorConstructor {
     this.reflectionFilters = reflectionFilters;
   }
 
+  /**
+   * Check if the class can be instantiated by Unsafe allocator. If the instance has interface or abstract modifiers
+   * return an exception message.
+   * @param c instance of the class to be checked
+   * @return if instantiable {@code null}, else a non-{@code null} exception message
+   */
+  static String checkInstantiable(Class<?> c) {
+    int modifiers = c.getModifiers();
+    if (Modifier.isInterface(modifiers)) {
+      return "Interfaces can't be instantiated! Register an InstanceCreator "
+          + "or a TypeAdapter for this type. Interface name: " + c.getName();
+    }
+    if (Modifier.isAbstract(modifiers)) {
+      return "Abstract classes can't be instantiated! Register an InstanceCreator "
+          + "or a TypeAdapter for this type. Class name: " + c.getName();
+    }
+    return null;
+  }
+
   public <T> ObjectConstructor<T> get(TypeToken<T> typeToken) {
     final Type type = typeToken.getType();
     final Class<? super T> rawType = typeToken.getRawType();
@@ -110,7 +129,7 @@ public final class ConstructorConstructor {
 
     // Check whether type is instantiable; otherwise ReflectionAccessFilter recommendation
     // of adjusting filter suggested below is irrelevant since it would not solve the problem
-    final String exceptionMessage = UnsafeAllocator.checkInstantiable(rawType);
+    final String exceptionMessage = checkInstantiable(rawType);
     if (exceptionMessage != null) {
       return new ObjectConstructor<T>() {
         @Override public T construct() {
@@ -342,11 +361,10 @@ public final class ConstructorConstructor {
   private <T> ObjectConstructor<T> newUnsafeAllocator(final Class<? super T> rawType) {
     if (useJdkUnsafe) {
       return new ObjectConstructor<T>() {
-        private final UnsafeAllocator unsafeAllocator = UnsafeAllocator.create();
         @Override public T construct() {
           try {
             @SuppressWarnings("unchecked")
-            T newInstance = (T) unsafeAllocator.newInstance(rawType);
+            T newInstance = (T) UnsafeAllocator.INSTANCE.newInstance(rawType);
             return newInstance;
           } catch (Exception e) {
             throw new RuntimeException(("Unable to create instance of " + rawType + ". "
diff --git a/gson/src/main/java/com/google/gson/internal/UnsafeAllocator.java b/gson/src/main/java/com/google/gson/internal/UnsafeAllocator.java
index 429bac6b..fae6f802 100644
--- a/gson/src/main/java/com/google/gson/internal/UnsafeAllocator.java
+++ b/gson/src/main/java/com/google/gson/internal/UnsafeAllocator.java
@@ -20,7 +20,6 @@ import java.io.ObjectInputStream;
 import java.io.ObjectStreamClass;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 
 /**
  * Do sneaky things to allocate objects without invoking their constructors.
@@ -32,37 +31,20 @@ public abstract class UnsafeAllocator {
   public abstract <T> T newInstance(Class<T> c) throws Exception;
 
   /**
-   * Check if the class can be instantiated by Unsafe allocator. If the instance has interface or abstract modifiers
-   * return an exception message.
-   * @param c instance of the class to be checked
-   * @return if instantiable {@code null}, else a non-{@code null} exception message
-   */
-  static String checkInstantiable(Class<?> c) {
-    int modifiers = c.getModifiers();
-    if (Modifier.isInterface(modifiers)) {
-      return "Interfaces can't be instantiated! Register an InstanceCreator "
-          + "or a TypeAdapter for this type. Interface name: " + c.getName();
-    }
-    if (Modifier.isAbstract(modifiers)) {
-      return "Abstract classes can't be instantiated! Register an InstanceCreator "
-          + "or a TypeAdapter for this type. Class name: " + c.getName();
-    }
-    return null;
-  }
-
-  /**
    * Asserts that the class is instantiable. This check should have already occurred
    * in {@link ConstructorConstructor}; this check here acts as safeguard since trying
    * to use Unsafe for non-instantiable classes might crash the JVM on some devices.
    */
   private static void assertInstantiable(Class<?> c) {
-    String exceptionMessage = checkInstantiable(c);
+    String exceptionMessage = ConstructorConstructor.checkInstantiable(c);
     if (exceptionMessage != null) {
       throw new AssertionError("UnsafeAllocator is used for non-instantiable type: " + exceptionMessage);
     }
   }
 
-  public static UnsafeAllocator create() {
+  public static final UnsafeAllocator INSTANCE = create();
+
+  private static UnsafeAllocator create() {
     // try JVM
     // public class Unsafe {
     //   public Object allocateInstance(Class<?> type);
diff --git a/gson/src/test/java/com/google/gson/internal/UnsafeAllocatorInstantiationTest.java b/gson/src/test/java/com/google/gson/internal/UnsafeAllocatorInstantiationTest.java
index e3ce147e..54d0a506 100644
--- a/gson/src/test/java/com/google/gson/internal/UnsafeAllocatorInstantiationTest.java
+++ b/gson/src/test/java/com/google/gson/internal/UnsafeAllocatorInstantiationTest.java
@@ -37,9 +37,8 @@ public final class UnsafeAllocatorInstantiationTest extends TestCase {
    * to instantiate an interface
    */
   public void testInterfaceInstantiation() throws Exception {
-    UnsafeAllocator unsafeAllocator = UnsafeAllocator.create();
     try {
-      unsafeAllocator.newInstance(Interface.class);
+      UnsafeAllocator.INSTANCE.newInstance(Interface.class);
       fail();
     } catch (AssertionError e) {
       assertTrue(e.getMessage().startsWith("UnsafeAllocator is used for non-instantiable type"));
@@ -51,9 +50,8 @@ public final class UnsafeAllocatorInstantiationTest extends TestCase {
    * to instantiate an abstract class
    */
   public void testAbstractClassInstantiation() throws Exception {
-    UnsafeAllocator unsafeAllocator = UnsafeAllocator.create();
     try {
-      unsafeAllocator.newInstance(AbstractClass.class);
+      UnsafeAllocator.INSTANCE.newInstance(AbstractClass.class);
       fail();
     } catch (AssertionError e) {
       assertTrue(e.getMessage().startsWith("UnsafeAllocator is used for non-instantiable type"));
@@ -64,8 +62,7 @@ public final class UnsafeAllocatorInstantiationTest extends TestCase {
    * Ensure that no exception is thrown when trying to instantiate a concrete class
    */
   public void testConcreteClassInstantiation() throws Exception {
-    UnsafeAllocator unsafeAllocator = UnsafeAllocator.create();
-    ConcreteClass instance = unsafeAllocator.newInstance(ConcreteClass.class);
+    ConcreteClass instance = UnsafeAllocator.INSTANCE.newInstance(ConcreteClass.class);
     assertNotNull(instance);
   }
 }