ensure RecursionManager internal consistency by not depending on user key hashCode being constant (many EA exceptions)

platform/util/src/com/intellij/openapi/util/RecursionManager.java

@@ -75,7 +75,7 @@ public class RecursionManager {
   public static RecursionGuard createGuard(@NonNls final String id) {
     return new RecursionGuard() {
       @Override
-      public <T> T doPreventingRecursion(@NotNull Object key, boolean memoize, Computable<T> computation) {
+      public <T> T doPreventingRecursion(@NotNull Object key, boolean memoize, @NotNull Computable<T> computation) {
         MyKey realKey = new MyKey(id, key);
         final CalculationStack stack = ourStack.get();
 
@@ -101,8 +101,6 @@ public class RecursionManager {
           }
         }
 
-        int oldHash = realKey.hashCode();
-
         final int sizeBefore = stack.progressMap.size();
         stack.beforeComputation(realKey);
         final int sizeAfter = stack.progressMap.size();
@@ -122,17 +120,15 @@ public class RecursionManager {
             stack.afterComputation(realKey, sizeBefore, sizeAfter);
           }
           catch (Throwable e) {
+            //noinspection ThrowFromFinallyBlock
             throw new RuntimeException("Throwable in afterComputation", e);
           }
 
           stack.checkDepth("4");
-
-          if (oldHash != realKey.hashCode()) {
-            throw new AssertionError("Object has changed its hashCode: " + key);
-          }
         }
       }
 
+      @NotNull
       @Override
       public StackStamp markStack() {
         final int stamp = ourStack.get().reentrancyCount;
@@ -144,6 +140,7 @@ public class RecursionManager {
         };
       }
 
+      @NotNull
       @Override
       public List<Object> currentStack() {
         ArrayList<Object> result = new ArrayList<Object>();
@@ -168,8 +165,17 @@ public class RecursionManager {
   }
 
   private static class MyKey extends Pair<String, Object> {
-    public MyKey(String first, Object second) {
-      super(first, second);
+    private int myHashCode;
+
+    public MyKey(String guardId, Object userObject) {
+      super(guardId, userObject);
+      // remember user object hashCode to ensure our internal maps consistency
+      myHashCode = guardId.hashCode() * 31 + userObject.hashCode();
+    }
+
+    @Override
+    public int hashCode() {
+      return myHashCode;
     }
   }
 
@@ -266,10 +272,10 @@ public class RecursionManager {
       if (depth == 0) {
         intermediateCache.clear();
         if (!key2ReentrancyDuringItsCalculation.isEmpty()) {
-          LOG.error("non-empty key2ReentrancyDuringItsCalculation: " + new HashMap(key2ReentrancyDuringItsCalculation));
+          LOG.error("non-empty key2ReentrancyDuringItsCalculation: " + new HashMap<MyKey, MyKey>(key2ReentrancyDuringItsCalculation));
         }
         if (!toMemoize.isEmpty()) {
-          LOG.error("non-empty toMemoize: " + new HashSet(toMemoize));
+          LOG.error("non-empty toMemoize: " + new HashSet<MyKey>(toMemoize));
         }
       }
 

platform/util/testSrc/com/intellij/openapi/util/RecursionManagerTest.groovy

@@ -23,14 +23,14 @@ import junit.framework.TestCase;
 public class RecursionManagerTest extends TestCase {
   private final RecursionGuard myGuard = RecursionManager.createGuard("RecursionManagerTest");
   
-  def prevent(String key, boolean memoize = true, Closure c) {
+  def prevent(Object key, boolean memoize = true, Closure c) {
     myGuard.doPreventingRecursion(key, memoize, c as Computable)
   }
 
   public void testPreventRecursion() {
-    assert "foo-return" == prevent("foo") {
+    assert "foo-return" == prevent(["foo"]) {
       assert "bar-return" == prevent("bar") {
-        assert null == prevent("foo") { "foo-return" }
+        assert null == prevent(["foo"]) { "foo-return" }
         return "bar-return"
       }
       return "foo-return"
@@ -156,4 +156,27 @@ public class RecursionManagerTest extends TestCase {
     assert System.currentTimeMillis() - start < 10000
   }
 
+  public void "test changing hash code doesn't crash RecursionManager"() {
+    def key = ["b"]
+    prevent(key) {
+      key << "a"
+    }
+  }
+
+  public void "test exception from hashCode on exiting"() {
+    boolean fail = false
+    Object key = new Object() {
+      @Override
+      int hashCode() {
+        if (fail) {
+          throw new RuntimeException()
+        }
+        return super.hashCode()
+      }
+    }
+    prevent(key) {
+      fail = true
+    }
+  }
+
 }