Merge pull request google#961 from google/moe_writing_branch_from_57e…

…87fedb5af85511af7247d2e58781c176e2333

Merge internal changes

build.xml

@@ -120,8 +120,8 @@
         <pathelement location="${build.dir}/dist/guice-${version}.jar"/>
         <pathelement location="lib/javax.inject.jar"/>
         <pathelement location="lib/aopalliance.jar"/>
-        <pathelement location="lib/guava-16.0.1.jar"/>
-        <pathelement location="lib/build/guava-testlib-16.0.1.jar"/>
+        <pathelement location="lib/guava-18.0.jar"/>
+        <pathelement location="lib/build/guava-testlib-18.0.jar"/>
         <pathelement location="lib/build/junit.jar"/>
         <pathelement location="lib/build/servlet-api-2.5.jar"/>
         <pathelement location="lib/build/easymock.jar"/>

core/src/com/google/inject/internal/BindingImpl.java

@@ -16,7 +16,7 @@
 
 package com.google.inject.internal;
 
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
 import com.google.inject.Binding;
 import com.google.inject.Key;
 import com.google.inject.Provider;
@@ -106,7 +106,7 @@ protected BindingImpl<T> withKey(Key<T> key) {
   }
 
   @Override public String toString() {
-    return Objects.toStringHelper(Binding.class)
+    return MoreObjects.toStringHelper(Binding.class)
         .add("key", key)
         .add("scope", scoping)
         .add("source", source)

core/src/com/google/inject/internal/ConstantFactory.java

@@ -16,7 +16,7 @@
 
 package com.google.inject.internal;
 
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
 import com.google.inject.spi.Dependency;
 
 /**
@@ -36,7 +36,7 @@ public T get(Errors errors, InternalContext context, Dependency dependency, bool
   }
 
   public String toString() {
-    return Objects.toStringHelper(ConstantFactory.class)
+    return MoreObjects.toStringHelper(ConstantFactory.class)
         .add("value", initializable)
         .toString();
   }

core/src/com/google/inject/internal/ConstructorBindingImpl.java

@@ -19,6 +19,7 @@
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.inject.internal.Annotations.findScopeAnnotation;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Binder;
@@ -218,7 +219,7 @@ public void applyTo(Binder binder) {
   }
 
   @Override public String toString() {
-    return Objects.toStringHelper(ConstructorBinding.class)
+    return MoreObjects.toStringHelper(ConstructorBinding.class)
         .add("key", getKey())
         .add("source", getSource())
         .add("scope", getScoping())

core/src/com/google/inject/internal/CycleDetectingLock.java

@@ -2,7 +2,6 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
-import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.ListMultimap;
@@ -86,9 +85,10 @@ class CycleDetectingLockFactory<ID> {
      * Same lock can be added for several threads in case all of them are trying to
      * take it.
      *
-     * Guarded by {@code this}.
+     * Guarded by {@code CycleDetectingLockFactory.class}.
      */
-    private Map<Long, ReentrantCycleDetectingLock> lockThreadIsWaitingOn = Maps.newHashMap();
+    private static Map<Long, ReentrantCycleDetectingLock<?>> lockThreadIsWaitingOn =
+        Maps.newHashMap();
 
     /**
      * Lists locks that thread owns.
@@ -104,28 +104,30 @@ class CycleDetectingLockFactory<ID> {
      * Same lock can only be present several times for the same thread as locks are
      * reentrant. Lock can not be owned by several different threads as the same time.
      *
-     * Guarded by {@code this}.
+     * Guarded by {@code CycleDetectingLockFactory.class}.
      */
-    private final Multimap<Long, ReentrantCycleDetectingLock> locksOwnedByThread =
+    private static final Multimap<Long, ReentrantCycleDetectingLock<?>> locksOwnedByThread =
         LinkedHashMultimap.create();
 
     /**
      * Creates new lock within this factory context. We can guarantee that locks created by
      * the same factory would not deadlock.
      *
-     * @param newLockId lock id that would be used to report lock cycles if detected
+     * @param userLockId lock id that would be used to report lock cycles if detected
      */
-    CycleDetectingLock<ID> create(ID newLockId) {
-      return new ReentrantCycleDetectingLock(newLockId, new ReentrantLock());
+    CycleDetectingLock<ID> create(ID userLockId) {
+      return new ReentrantCycleDetectingLock<ID>(this, userLockId, new ReentrantLock());
     }
 
     /** The implementation for {@link CycleDetectingLock}. */
-    class ReentrantCycleDetectingLock implements CycleDetectingLock<ID> {
+    static class ReentrantCycleDetectingLock<ID> implements CycleDetectingLock<ID> {
 
       /** Underlying lock used for actual waiting when no potential deadlocks are detected. */
       private final Lock lockImplementation;
       /** User id for this lock. */
       private final ID userLockId;
+      /** Factory that was used to create this lock. */
+      private final CycleDetectingLockFactory<ID> lockFactory;
       /**
        * Thread id for the thread that owned this lock. Nullable.
        * Guarded by {@code CycleDetectingLockFactory.this}.
@@ -137,15 +139,17 @@ class ReentrantCycleDetectingLock implements CycleDetectingLock<ID> {
        */
       private int lockReentranceCount = 0;
 
-      ReentrantCycleDetectingLock(ID userLockId, Lock lockImplementation) {
+      ReentrantCycleDetectingLock(CycleDetectingLockFactory<ID> lockFactory,
+          ID userLockId, Lock lockImplementation) {
+        this.lockFactory = lockFactory;
         this.userLockId = Preconditions.checkNotNull(userLockId, "userLockId");
         this.lockImplementation = Preconditions.checkNotNull(
             lockImplementation, "lockImplementation");
       }
 
       @Override public ListMultimap<Long, ID> lockOrDetectPotentialLocksCycle() {
         final long currentThreadId = Thread.currentThread().getId();
-        synchronized (CycleDetectingLockFactory.this) {
+        synchronized (CycleDetectingLockFactory.class) {
           checkState();
           ListMultimap<Long, ID> locksInCycle = detectPotentialLocksCycle();
           if (!locksInCycle.isEmpty()) {
@@ -159,7 +163,7 @@ class ReentrantCycleDetectingLock implements CycleDetectingLock<ID> {
         // this may be blocking, but we don't expect it to cause a deadlock
         lockImplementation.lock();
 
-        synchronized (CycleDetectingLockFactory.this) {
+        synchronized (CycleDetectingLockFactory.class) {
           // current thread is no longer waiting on this lock
           lockThreadIsWaitingOn.remove(currentThreadId);
           checkState();
@@ -176,7 +180,7 @@ class ReentrantCycleDetectingLock implements CycleDetectingLock<ID> {
 
       @Override public void unlock() {
         final long currentThreadId = Thread.currentThread().getId();
-        synchronized (CycleDetectingLockFactory.this) {
+        synchronized (CycleDetectingLockFactory.class) {
           checkState();
           Preconditions.checkState(lockOwnerThreadId != null,
               "Thread is trying to unlock a lock that is not locked");
@@ -270,15 +274,20 @@ public List<ID> get() {
       private List<ID> getAllLockIdsAfter(long threadId, ReentrantCycleDetectingLock lock) {
         List<ID> ids = Lists.newArrayList();
         boolean found = false;
-        Collection<ReentrantCycleDetectingLock> ownedLocks = locksOwnedByThread.get(threadId);
+        Collection<ReentrantCycleDetectingLock<?>> ownedLocks = locksOwnedByThread.get(threadId);
         Preconditions.checkNotNull(ownedLocks,
             "Internal error: No locks were found taken by a thread");
         for (ReentrantCycleDetectingLock ownedLock : ownedLocks) {
           if (ownedLock == lock) {
             found = true;
           }
-          if (found) {
-            ids.add(ownedLock.userLockId);
+          if (found && ownedLock.lockFactory == this.lockFactory) {
+            // All locks are stored in a shared map therefore there is no way to
+            // enforce type safety. We know that our cast is valid as we check for a lock's
+            // factory. If the lock was generated by the
+            // same factory it has to have same type as the current lock.
+            @SuppressWarnings("unchecked") ID userLockId = (ID) ownedLock.userLockId;
+            ids.add(userLockId);
           }
         }
         Preconditions.checkState(found, "Internal error: We can not find locks that "
@@ -289,12 +298,11 @@ private List<ID> getAllLockIdsAfter(long threadId, ReentrantCycleDetectingLock l
       @Override public String toString() {
         // copy is made to prevent a data race
         // no synchronization is used, potentially stale data, should be good enough
-        Long localLockOwnerThreadId = this.lockOwnerThreadId;
-        if (localLockOwnerThreadId != null) {
-          return String.format("CycleDetectingLock[%s][locked by %s]",
-              userLockId, localLockOwnerThreadId);
+        Long threadId = this.lockOwnerThreadId;
+        if (threadId != null) {
+          return String.format("%s[%s][locked by Id=%d]", super.toString(), userLockId, threadId);
         } else {
-          return String.format("CycleDetectingLock[%s][unlocked]", userLockId);
+          return String.format("%s[%s][unlocked]", super.toString(), userLockId);
         }
       }
     }

core/src/com/google/inject/internal/Errors.java

@@ -24,7 +24,6 @@
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Ordering;
-import com.google.common.collect.Sets;
 import com.google.inject.ConfigurationException;
 import com.google.inject.CreationException;
 import com.google.inject.Guice;
@@ -56,6 +55,7 @@
 import java.lang.reflect.Type;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Formatter;
 import java.util.List;
 import java.util.Map;
@@ -84,7 +84,7 @@ public final class Errors implements Serializable {
   private static final Logger logger = Logger.getLogger(Guice.class.getName());
 
   private static final Set<Dependency<?>> warnedDependencies =
-      Sets.newSetFromMap(new ConcurrentHashMap<Dependency<?>, Boolean>());
+      Collections.newSetFromMap(new ConcurrentHashMap<Dependency<?>, Boolean>());
 
 
   /**

core/src/com/google/inject/internal/ExposedBindingImpl.java

@@ -16,7 +16,7 @@
 
 package com.google.inject.internal;
 
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableSet;
 import com.google.inject.Binder;
 import com.google.inject.Injector;
@@ -51,7 +51,7 @@ public PrivateElements getPrivateElements() {
   }
 
   @Override public String toString() {
-    return Objects.toStringHelper(ExposedBinding.class)
+    return MoreObjects.toStringHelper(ExposedBinding.class)
         .add("key", getKey())
         .add("source", getSource())
         .add("privateElements", privateElements)

core/src/com/google/inject/internal/FactoryProxy.java

@@ -16,7 +16,7 @@
 
 package com.google.inject.internal;
 
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
 import com.google.inject.Key;
 import com.google.inject.internal.InjectorImpl.JitLimitation;
 import com.google.inject.spi.Dependency;
@@ -60,7 +60,7 @@ public T get(Errors errors, InternalContext context, Dependency<?> dependency, b
   }
 
   @Override public String toString() {
-    return Objects.toStringHelper(FactoryProxy.class)
+    return MoreObjects.toStringHelper(FactoryProxy.class)
         .add("key", key)
         .add("provider", targetFactory)
         .toString();