From 31b1e22561d5db89abfd09a8ecd9932d20f5eda3 Mon Sep 17 00:00:00 2001

From: Mikolaj Izdebski <mizdebsk@redhat.com>

Date: Mon, 18 May 2015 12:05:45 +0200

Subject: [PATCH] Revert "Some work on issue 910 -- ensure that anonymous keys

 & typeliterals don't"

This reverts commit 825f8c1df885b9d7643a9e18e336984f0138edaf.

---

 .../com/google/inject/internal/InjectorImpl.java   |  1 -

 core/src/com/google/inject/internal/MoreTypes.java | 37 ++-----------------

 core/src/com/google/inject/spi/Dependency.java     |  3 +-

 core/src/com/google/inject/spi/Elements.java       | 14 +++----

 core/test/com/google/inject/Asserts.java           | 42 ---------------------

 core/test/com/google/inject/KeyTest.java           | 43 ----------------------

 .../com/google/inject/internal/WeakKeySetTest.java | 11 +++++-

 .../google/inject/internal/WeakKeySetUtils.java    | 42 +++++++++++++++++++++

 .../google/inject/multibindings/MapBinderTest.java |  3 +-

 .../inject/multibindings/OptionalBinderTest.java   |  3 +-

 10 files changed, 62 insertions(+), 137 deletions(-)

diff --git a/core/src/com/google/inject/internal/InjectorImpl.java b/core/src/com/google/inject/internal/InjectorImpl.java

index 54ce8a3..d260046 100644

--- a/core/src/com/google/inject/internal/InjectorImpl.java

+++ b/core/src/com/google/inject/internal/InjectorImpl.java

@@ -801,7 +801,6 @@ final class InjectorImpl implements Injector, Lookups {

       throw errors.childBindingAlreadySet(key, sources).toException();

     }

-    key = MoreTypes.canonicalizeKey(key); // before storing the key long-term, canonicalize it.

     BindingImpl<T> binding = createJustInTimeBinding(key, errors, jitDisabled, jitType);

     state.parent().blacklist(key, state, binding.getSource());

     jitBindings.put(key, binding);

diff --git a/core/src/com/google/inject/internal/MoreTypes.java b/core/src/com/google/inject/internal/MoreTypes.java

index bdf6029..12a7625 100644

--- a/core/src/com/google/inject/internal/MoreTypes.java

+++ b/core/src/com/google/inject/internal/MoreTypes.java

@@ -23,7 +23,6 @@ import static com.google.common.base.Preconditions.checkNotNull;

 import com.google.common.base.Objects;

 import com.google.common.collect.ImmutableMap;

 import com.google.inject.ConfigurationException;

-import com.google.inject.Key;

 import com.google.inject.TypeLiteral;

 import com.google.inject.util.Types;

@@ -65,25 +64,6 @@ public class MoreTypes {

           .build();

   /**

-   * Returns a key that doesn't hold any references to parent classes.

-   * This is necessary for anonymous keys, so ensure we don't hold a ref

-   * to the containing module (or class) forever.

-   */

-  public static <T> Key<T> canonicalizeKey(Key<T> key) {

-    // If we know this isn't a subclass, return as-is.

-    // Otherwise, recreate the key to avoid the subclass 

-    if (key.getClass() == Key.class) {

-      return key;

-    } else if (key.getAnnotation() != null) {

-      return Key.get(key.getTypeLiteral(), key.getAnnotation());

-    } else if (key.getAnnotationType() != null) {

-      return Key.get(key.getTypeLiteral(), key.getAnnotationType());

-    } else {

-      return Key.get(key.getTypeLiteral());

-    }

-  }

-

-  /**

    * Returns an type that's appropriate for use in a key.

    *

    * 
If the raw type of {@code typeLiteral} is a {@code javax.inject.Provider}, this returns a

@@ -113,20 +93,9 @@ public class MoreTypes {

     @SuppressWarnings("unchecked")

     TypeLiteral<T> wrappedPrimitives = (TypeLiteral<T>) PRIMITIVE_TO_WRAPPER.get(typeLiteral);

-    if (wrappedPrimitives != null) {

-      return wrappedPrimitives;

-    }

-

-    // If we know this isn't a subclass, return as-is.

-    if (typeLiteral.getClass() == TypeLiteral.class) {

-      return typeLiteral;

-    }

-

-    // recreate the TypeLiteral to avoid anonymous TypeLiterals from holding refs to their

-    // surrounding classes.

-    @SuppressWarnings("unchecked")

-    TypeLiteral<T> recreated = (TypeLiteral<T>) TypeLiteral.get(typeLiteral.getType());

-    return recreated;

+    return wrappedPrimitives != null

+        ? wrappedPrimitives

+        : typeLiteral;

   }

   /**

diff --git a/core/src/com/google/inject/spi/Dependency.java b/core/src/com/google/inject/spi/Dependency.java

index f86e255..c51d87c 100644

--- a/core/src/com/google/inject/spi/Dependency.java

+++ b/core/src/com/google/inject/spi/Dependency.java

@@ -22,7 +22,6 @@ import com.google.common.base.Objects;

 import com.google.common.collect.ImmutableSet;

 import com.google.common.collect.Lists;

 import com.google.inject.Key;

-import com.google.inject.internal.MoreTypes;

 import java.util.List;

 import java.util.Set;

@@ -55,7 +54,7 @@ public final class Dependency<T> {

    * nullable.

    */

   public static <T> Dependency<T> get(Key<T> key) {

-    return new Dependency<T>(null, MoreTypes.canonicalizeKey(key), true, -1);

+    return new Dependency<T>(null, key, true, -1);

   }

   /**

diff --git a/core/src/com/google/inject/spi/Elements.java b/core/src/com/google/inject/spi/Elements.java

index 9348276..f5bbe89 100644

--- a/core/src/com/google/inject/spi/Elements.java

+++ b/core/src/com/google/inject/spi/Elements.java

@@ -44,7 +44,6 @@ import com.google.inject.internal.ConstantBindingBuilderImpl;

 import com.google.inject.internal.Errors;

 import com.google.inject.internal.ExposureBuilder;

 import com.google.inject.internal.InternalFlags.IncludeStackTraceOption;

-import com.google.inject.internal.MoreTypes;

 import com.google.inject.internal.PrivateElementsImpl;

 import com.google.inject.internal.ProviderMethodsModule;

 import com.google.inject.internal.util.SourceProvider;

@@ -243,14 +242,13 @@ public final class Elements {

     @Override

     public <T> void requestInjection(TypeLiteral<T> type, T instance) {

-      elements.add(new InjectionRequest<T>(getElementSource(), MoreTypes.canonicalizeForKey(type),

-          instance));

+      elements.add(new InjectionRequest<T>(getElementSource(), type, instance));

     }

     @Override

     public <T> MembersInjector<T> getMembersInjector(final TypeLiteral<T> typeLiteral) {

-      final MembersInjectorLookup<T> element = new MembersInjectorLookup<T>(getElementSource(),

-          MoreTypes.canonicalizeForKey(typeLiteral));

+      final MembersInjectorLookup<T> element

+          = new MembersInjectorLookup<T>(getElementSource(), typeLiteral);

       elements.add(element);

       return element.getMembersInjector();

     }

@@ -372,8 +370,7 @@ public final class Elements {

     }

     public <T> AnnotatedBindingBuilder<T> bind(Key<T> key) {

-      BindingBuilder<T> builder =

-          new BindingBuilder<T>(this, elements, getElementSource(), MoreTypes.canonicalizeKey(key));

+      BindingBuilder<T> builder = new BindingBuilder<T>(this, elements, getElementSource(), key);

       return builder;

     }

@@ -483,8 +480,7 @@ public final class Elements {

         };

       }

-      ExposureBuilder<T> builder =

-          new ExposureBuilder<T>(this, getElementSource(), MoreTypes.canonicalizeKey(key));

+      ExposureBuilder<T> builder = new ExposureBuilder<T>(this, getElementSource(), key);

       privateElements.addExposureBuilder(builder);

       return builder;

     }

diff --git a/core/test/com/google/inject/Asserts.java b/core/test/com/google/inject/Asserts.java

index 6c63158..363e161 100644

--- a/core/test/com/google/inject/Asserts.java

+++ b/core/test/com/google/inject/Asserts.java

@@ -21,14 +21,12 @@ import static com.google.inject.internal.InternalFlags.IncludeStackTraceOption;

 import static com.google.inject.internal.InternalFlags.getIncludeStackTraceOption;

 import static junit.framework.Assert.assertEquals;

 import static junit.framework.Assert.assertNotNull;

-import static junit.framework.Assert.assertSame;

 import static junit.framework.Assert.assertTrue;

 import com.google.common.base.Function;

 import com.google.common.base.Joiner;

 import com.google.common.collect.ImmutableList;

 import com.google.common.collect.Iterables;

-import com.google.common.testing.GcFinalization;

 import junit.framework.Assert;

@@ -38,8 +36,6 @@ import java.io.IOException;

 import java.io.NotSerializableException;

 import java.io.ObjectInputStream;

 import java.io.ObjectOutputStream;

-import java.lang.ref.ReferenceQueue;

-import java.lang.ref.WeakReference;

 /**

  * @author jessewilson@google.com (Jesse Wilson)

@@ -163,42 +159,4 @@ public class Asserts {

     } catch (NotSerializableException expected) {

     }

   }

-

-  public static void awaitFullGc() {

-    // GcFinalization *should* do it, but doesn't work well in practice...

-    // so we put a second latch and wait for a ReferenceQueue to tell us.

-    ReferenceQueue<Object> queue = new ReferenceQueue<Object>();

-    WeakReference ref = new WeakReference<Object>(new Object(), queue);

-    GcFinalization.awaitFullGc();

-    try {

-      assertSame("queue didn't return ref in time", ref, queue.remove(5000));

-    } catch (IllegalArgumentException e) {

-      throw new RuntimeException(e);

-    } catch (InterruptedException e) {

-      throw new RuntimeException(e);

-    }

-  }

-

-  public static void awaitClear(WeakReference ref) {

-    // GcFinalization *should* do it, but doesn't work well in practice...

-    // so we put a second latch and wait for a ReferenceQueue to tell us.

-    Object data = ref.get();

-    ReferenceQueue<Object> queue = null;

-    WeakReference extraRef = null;

-    if (data != null) {

-      queue = new ReferenceQueue<Object>();

-      extraRef = new WeakReference<Object>(data, queue);

-      data = null;

-    }

-    GcFinalization.awaitClear(ref);

-    if (queue != null) {

-      try {

-        assertSame("queue didn't return ref in time", extraRef, queue.remove(5000));

-      } catch (IllegalArgumentException e) {

-        throw new RuntimeException(e);

-      } catch (InterruptedException e) {

-        throw new RuntimeException(e);

-      }

-    }

-  }

 }

diff --git a/core/test/com/google/inject/KeyTest.java b/core/test/com/google/inject/KeyTest.java

index d9dd943..c0401e0 100644

--- a/core/test/com/google/inject/KeyTest.java

+++ b/core/test/com/google/inject/KeyTest.java

@@ -19,12 +19,10 @@ package com.google.inject;

 import static com.google.inject.Asserts.assertContains;

 import static com.google.inject.Asserts.assertEqualsBothWays;

 import static com.google.inject.Asserts.assertNotSerializable;

-import static com.google.inject.Asserts.awaitClear;

 import static java.lang.annotation.RetentionPolicy.RUNTIME;

 import com.google.inject.name.Named;

 import com.google.inject.name.Names;

-import com.google.inject.spi.Dependency;

 import com.google.inject.util.Types;

 import junit.framework.TestCase;

@@ -33,15 +31,12 @@ import java.io.IOException;

 import java.lang.annotation.ElementType;

 import java.lang.annotation.Retention;

 import java.lang.annotation.Target;

-import java.lang.ref.WeakReference;

 import java.lang.reflect.Method;

 import java.lang.reflect.ParameterizedType;

 import java.lang.reflect.Type;

 import java.lang.reflect.TypeVariable;

-import java.util.ArrayList;

 import java.util.List;

 import java.util.Map;

-import java.util.concurrent.atomic.AtomicReference;

 /**

  * @author crazybob@google.com (Bob Lee)

@@ -280,42 +275,4 @@ public class KeyTest extends TestCase {

   @Marker

   class HasAnnotations {}

-  public void testAnonymousClassesDontHoldRefs() {

-    final AtomicReference<Provider<List<String>>> stringProvider =

-        new AtomicReference<Provider<List<String>>>();

-    final AtomicReference<Provider<List<Integer>>> intProvider =

-        new AtomicReference<Provider<List<Integer>>>();

-    final Object foo = new Object() {

-      @SuppressWarnings("unused") @Inject List<String> list;

-    };

-    Module module = new AbstractModule() {

-      @Override protected void configure() {

-        bind(new Key<List<String>>() {}).toInstance(new ArrayList<String>());

-        bind(new TypeLiteral<List<Integer>>() {}).toInstance(new ArrayList<Integer>());

-

-        stringProvider.set(getProvider(new Key<List<String>>() {}));

-        intProvider.set(binder().getProvider(Dependency.get(new Key<List<Integer>>() {})));

-

-        binder().requestInjection(new TypeLiteral<Object>() {}, foo);

-      }

-    };

-    WeakReference<Module> moduleRef = new WeakReference<Module>(module);

-    final Injector injector = Guice.createInjector(module);

-    module = null;

-    awaitClear(moduleRef); // Make sure anonymous keys & typeliterals don't hold the module.

-

-    Runnable runner = new Runnable() {

-      @Override public void run() {

-        injector.getInstance(new Key<Typed<String>>() {});

-        injector.getInstance(Key.get(new TypeLiteral<Typed<Integer>>() {}));

-      }

-    };

-    WeakReference<Runnable> runnerRef = new WeakReference<Runnable>(runner);

-    runner.run();

-    runner = null;

-    awaitClear(runnerRef); // also make sure anonymous keys & typeliterals don't hold for JITs

-  }

-

-  static class Typed<T> {}

-

 }

diff --git a/core/test/com/google/inject/internal/WeakKeySetTest.java b/core/test/com/google/inject/internal/WeakKeySetTest.java

index 4a81ebb..3797d88 100644

--- a/core/test/com/google/inject/internal/WeakKeySetTest.java

+++ b/core/test/com/google/inject/internal/WeakKeySetTest.java

@@ -16,13 +16,13 @@

 package com.google.inject.internal;

-import static com.google.inject.Asserts.awaitClear;

-import static com.google.inject.Asserts.awaitFullGc;

 import static com.google.inject.internal.WeakKeySetUtils.assertBlacklisted;

 import static com.google.inject.internal.WeakKeySetUtils.assertInSet;

 import static com.google.inject.internal.WeakKeySetUtils.assertNotBlacklisted;

 import static com.google.inject.internal.WeakKeySetUtils.assertNotInSet;

 import static com.google.inject.internal.WeakKeySetUtils.assertSourceNotInSet;

+import static com.google.inject.internal.WeakKeySetUtils.awaitClear;

+import static com.google.inject.internal.WeakKeySetUtils.awaitFullGc;

 import com.google.common.collect.ImmutableList;

 import com.google.common.collect.ImmutableMap;

@@ -34,6 +34,13 @@ import com.google.inject.Injector;

 import com.google.inject.Key;

 import com.google.inject.Scope;

 import com.google.inject.TypeLiteral;

+import com.google.inject.internal.BindingImpl;

+import com.google.inject.internal.Errors;

+/*if[AOP]*/

+import com.google.inject.internal.MethodAspect;

+/*end[AOP]*/

+import com.google.inject.internal.State;

+import com.google.inject.internal.WeakKeySet;

 import com.google.inject.spi.ModuleAnnotatedMethodScannerBinding;

 import com.google.inject.spi.ProvisionListenerBinding;

 import com.google.inject.spi.ScopeBinding;

diff --git a/core/test/com/google/inject/internal/WeakKeySetUtils.java b/core/test/com/google/inject/internal/WeakKeySetUtils.java

index b023aa1..bab9e92 100644

--- a/core/test/com/google/inject/internal/WeakKeySetUtils.java

+++ b/core/test/com/google/inject/internal/WeakKeySetUtils.java

@@ -18,11 +18,15 @@ import static junit.framework.Assert.assertEquals;

 import static junit.framework.Assert.assertFalse;

 import static junit.framework.Assert.assertNotNull;

 import static junit.framework.Assert.assertNull;

+import static junit.framework.Assert.assertSame;

 import static junit.framework.Assert.assertTrue;

+import com.google.common.testing.GcFinalization;

 import com.google.inject.Injector;

 import com.google.inject.Key;

+import java.lang.ref.ReferenceQueue;

+import java.lang.ref.WeakReference;

 import java.util.Set;

 /**

@@ -34,6 +38,44 @@ public final class WeakKeySetUtils {

   private WeakKeySetUtils() {}

+  public static void awaitFullGc() {

+    // GcFinalization *should* do it, but doesn't work well in practice...

+    // so we put a second latch and wait for a ReferenceQueue to tell us.

+    ReferenceQueue<Object> queue = new ReferenceQueue<Object>();

+    WeakReference ref = new WeakReference<Object>(new Object(), queue);

+    GcFinalization.awaitFullGc();

+    try {

+      assertSame("queue didn't return ref in time", ref, queue.remove(5000));

+    } catch (IllegalArgumentException e) {

+      throw new RuntimeException(e);

+    } catch (InterruptedException e) {

+      throw new RuntimeException(e);

+    }

+  }

+

+  public static void awaitClear(WeakReference ref) {

+    // GcFinalization *should* do it, but doesn't work well in practice...

+    // so we put a second latch and wait for a ReferenceQueue to tell us.

+    Object data = ref.get();

+    ReferenceQueue<Object> queue = null;

+    WeakReference extraRef = null;

+    if (data != null) {

+      queue = new ReferenceQueue<Object>();

+      extraRef = new WeakReference<Object>(data, queue);

+      data = null;

+    }

+    GcFinalization.awaitClear(ref);

+    if (queue != null) {

+      try {

+        assertSame("queue didn't return ref in time", extraRef, queue.remove(5000));

+      } catch (IllegalArgumentException e) {

+        throw new RuntimeException(e);

+      } catch (InterruptedException e) {

+        throw new RuntimeException(e);

+      }

+    }

+  }

+

   public static void assertBlacklisted(Injector injector, Key key) {

     assertBlacklistState(injector, key, true);

   }

diff --git a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java

index 4206521..849993f 100644

--- a/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java

+++ b/extensions/multibindings/test/com/google/inject/multibindings/MapBinderTest.java

@@ -32,7 +32,6 @@ import com.google.common.collect.Iterables;

 import com.google.common.collect.Maps;

 import com.google.common.collect.Sets;

 import com.google.inject.AbstractModule;

-import com.google.inject.Asserts;

 import com.google.inject.Binding;

 import com.google.inject.BindingAnnotation;

 import com.google.inject.ConfigurationException;

@@ -1026,7 +1025,7 @@ public class MapBinderTest extends TestCase {

     // Clear the ref, GC, and ensure that we are no longer blacklisting.

     childInjector = null;

-    Asserts.awaitClear(weakRef);

+    WeakKeySetUtils.awaitClear(weakRef);

     WeakKeySetUtils.assertNotBlacklisted(parentInjector, mapKey);

   }

 }

diff --git a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java

index f3c9f63..d0a239a 100644

--- a/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java

+++ b/extensions/multibindings/test/com/google/inject/multibindings/OptionalBinderTest.java

@@ -30,7 +30,6 @@ import com.google.common.collect.Iterables;

 import com.google.common.collect.Lists;

 import com.google.common.collect.Sets;

 import com.google.inject.AbstractModule;

-import com.google.inject.Asserts;

 import com.google.inject.Binding;

 import com.google.inject.BindingAnnotation;

 import com.google.inject.CreationException;

@@ -1204,7 +1203,7 @@ public class OptionalBinderTest extends TestCase {

    // Clear the ref, GC, and ensure that we are no longer blacklisting.

    childInjector = null;

-   Asserts.awaitClear(weakRef);

+   WeakKeySetUtils.awaitClear(weakRef);

    WeakKeySetUtils.assertNotBlacklisted(parentInjector, Key.get(Integer.class));

  }

-- 

2.1.0