From d10a4f7b71d6718971e30f25b5db742dbc2371f8 Mon Sep 17 00:00:00 2001 From: Mikolaj Izdebski Date: Mon, 18 May 2015 12:10:02 +0200 Subject: [PATCH] Temporarly revert upstream commit which introduces regression --- ...-on-issue-910-ensure-that-anonymous-.patch | 456 ++++++++++++++++++ google-guice.spec | 3 + 2 files changed, 459 insertions(+) create mode 100644 0001-Revert-Some-work-on-issue-910-ensure-that-anonymous-.patch diff --git a/0001-Revert-Some-work-on-issue-910-ensure-that-anonymous-.patch b/0001-Revert-Some-work-on-issue-910-ensure-that-anonymous-.patch new file mode 100644 index 0000000..77abf33 --- /dev/null +++ b/0001-Revert-Some-work-on-issue-910-ensure-that-anonymous-.patch @@ -0,0 +1,456 @@ +From 31b1e22561d5db89abfd09a8ecd9932d20f5eda3 Mon Sep 17 00:00:00 2001 +From: Mikolaj Izdebski +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 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 Key canonicalizeKey(Key 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 wrappedPrimitives = (TypeLiteral) 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 recreated = (TypeLiteral) 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 { + * nullable. + */ + public static Dependency get(Key key) { +- return new Dependency(null, MoreTypes.canonicalizeKey(key), true, -1); ++ return new Dependency(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 void requestInjection(TypeLiteral type, T instance) { +- elements.add(new InjectionRequest(getElementSource(), MoreTypes.canonicalizeForKey(type), +- instance)); ++ elements.add(new InjectionRequest(getElementSource(), type, instance)); + } + + @Override + public MembersInjector getMembersInjector(final TypeLiteral typeLiteral) { +- final MembersInjectorLookup element = new MembersInjectorLookup(getElementSource(), +- MoreTypes.canonicalizeForKey(typeLiteral)); ++ final MembersInjectorLookup element ++ = new MembersInjectorLookup(getElementSource(), typeLiteral); + elements.add(element); + return element.getMembersInjector(); + } +@@ -372,8 +370,7 @@ public final class Elements { + } + + public AnnotatedBindingBuilder bind(Key key) { +- BindingBuilder builder = +- new BindingBuilder(this, elements, getElementSource(), MoreTypes.canonicalizeKey(key)); ++ BindingBuilder builder = new BindingBuilder(this, elements, getElementSource(), key); + return builder; + } + +@@ -483,8 +480,7 @@ public final class Elements { + }; + } + +- ExposureBuilder builder = +- new ExposureBuilder(this, getElementSource(), MoreTypes.canonicalizeKey(key)); ++ ExposureBuilder builder = new ExposureBuilder(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 queue = new ReferenceQueue(); +- WeakReference ref = new WeakReference(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 queue = null; +- WeakReference extraRef = null; +- if (data != null) { +- queue = new ReferenceQueue(); +- extraRef = new WeakReference(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>> stringProvider = +- new AtomicReference>>(); +- final AtomicReference>> intProvider = +- new AtomicReference>>(); +- final Object foo = new Object() { +- @SuppressWarnings("unused") @Inject List list; +- }; +- Module module = new AbstractModule() { +- @Override protected void configure() { +- bind(new Key>() {}).toInstance(new ArrayList()); +- bind(new TypeLiteral>() {}).toInstance(new ArrayList()); +- +- stringProvider.set(getProvider(new Key>() {})); +- intProvider.set(binder().getProvider(Dependency.get(new Key>() {}))); +- +- binder().requestInjection(new TypeLiteral() {}, foo); +- } +- }; +- WeakReference moduleRef = new WeakReference(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>() {}); +- injector.getInstance(Key.get(new TypeLiteral>() {})); +- } +- }; +- WeakReference runnerRef = new WeakReference(runner); +- runner.run(); +- runner = null; +- awaitClear(runnerRef); // also make sure anonymous keys & typeliterals don't hold for JITs +- } +- +- static class Typed {} +- + } +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 queue = new ReferenceQueue(); ++ WeakReference ref = new WeakReference(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 queue = null; ++ WeakReference extraRef = null; ++ if (data != null) { ++ queue = new ReferenceQueue(); ++ extraRef = new WeakReference(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 + diff --git a/google-guice.spec b/google-guice.spec index 1343337..98dd2cd 100644 --- a/google-guice.spec +++ b/google-guice.spec @@ -16,6 +16,8 @@ BuildArch: noarch Source0: %{name}-%{version}.tar.xz Source1: create-tarball.sh +Patch0: 0001-Revert-Some-work-on-issue-910-ensure-that-anonymous-.patch + # Rejected upstream: https://github.com/google/guice/issues/492 Patch100: https://raw.githubusercontent.com/sonatype/sisu-guice/master/PATCHES/GUICE_492_slf4j_logger_injection.patch # Forwarded upstream: https://github.com/google/guice/issues/618 @@ -175,6 +177,7 @@ This package provides %{summary}. %prep %setup -q -n %{name}-%{version} +%patch0 -p1 %patch100 -p1 %patch101 -p1