From 0e69ebb28866250af4e759e2f7383949a85b71f0 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 21 Jul 2015 13:00:59 +0300 Subject: [PATCH] Improve stack traces of exceptions thrown by LockBasedStorageManager Drop leading frames which only contain memoized functions' and lazy values' invokes, so that the first frame corresponds to the meaningful place in the code --- .../storage/LockBasedStorageManager.java | 52 ++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java b/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java index cfe33e80338..eea8886a4b6 100644 --- a/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java +++ b/core/util.runtime/src/org/jetbrains/kotlin/storage/LockBasedStorageManager.java @@ -24,6 +24,8 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.utils.UtilsPackage; import org.jetbrains.kotlin.utils.WrappedValues; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.Lock; @@ -224,7 +226,7 @@ public class LockBasedStorageManager implements StorageManager { @NotNull protected RecursionDetectedResult recursionDetectedDefault() { - throw new IllegalStateException("Recursive call in a lazy value under " + this); + throw sanitizeStackTrace(new IllegalStateException("Recursive call in a lazy value under " + this)); } private static class RecursionDetectedResult { @@ -376,7 +378,9 @@ public class LockBasedStorageManager implements StorageManager { lock.lock(); try { value = cache.get(input); - assert value != NotValue.COMPUTING : "Recursion detected on input: " + input + " under " + LockBasedStorageManager.this; + if (value == NotValue.COMPUTING) { + throw recursionDetected(input); + } if (value != null) return WrappedValues.unescapeExceptionOrNull(value); AssertionError error = null; @@ -390,8 +394,7 @@ public class LockBasedStorageManager implements StorageManager { // A seemingly obvious way to come about this case would be to declare a special exception class, but the problem is that // one memoized function is likely to (indirectly) call another, and if this second one throws this exception, we are screwed if (oldValue != NotValue.COMPUTING) { - error = new AssertionError("Race condition detected on input " + input + ". Old value is " + oldValue + - " under " + LockBasedStorageManager.this); + error = raceCondition(input, oldValue); throw error; } @@ -401,8 +404,9 @@ public class LockBasedStorageManager implements StorageManager { if (throwable == error) throw exceptionHandlingStrategy.handleException(throwable); Object oldValue = cache.put(input, WrappedValues.escapeThrowable(throwable)); - assert oldValue == NotValue.COMPUTING : "Race condition detected on input " + input + ". Old value is " + oldValue + - " under " + LockBasedStorageManager.this; + if (oldValue != NotValue.COMPUTING) { + throw raceCondition(input, oldValue); + } throw exceptionHandlingStrategy.handleException(throwable); } @@ -412,6 +416,21 @@ public class LockBasedStorageManager implements StorageManager { } } + @NotNull + private AssertionError recursionDetected(K input) { + return sanitizeStackTrace( + new AssertionError("Recursion detected on input: " + input + " under " + LockBasedStorageManager.this) + ); + } + + @NotNull + private AssertionError raceCondition(K input, Object oldValue) { + return sanitizeStackTrace( + new AssertionError("Race condition detected on input " + input + ". Old value is " + oldValue + + " under " + LockBasedStorageManager.this) + ); + } + @Override public boolean isComputed(K key) { Object value = cache.get(key); @@ -444,4 +463,25 @@ public class LockBasedStorageManager implements StorageManager { ) { return new LockBasedStorageManager(getPointOfConstruction(), newStrategy, base.lock); } + + @NotNull + private static T sanitizeStackTrace(@NotNull T throwable) { + String storagePackageName = LockBasedStorageManager.class.getPackage().getName(); + StackTraceElement[] stackTrace = throwable.getStackTrace(); + int size = stackTrace.length; + + int firstNonStorage = -1; + for (int i = 0; i < size; i++) { + // Skip everything (memoized functions and lazy values) from package org.jetbrains.kotlin.storage + if (!stackTrace[i].getClassName().startsWith(storagePackageName)) { + firstNonStorage = i; + break; + } + } + assert firstNonStorage >= 0 : "This method should only be called on exceptions created in LockBasedStorageManager"; + + List list = Arrays.asList(stackTrace).subList(firstNonStorage, size); + throwable.setStackTrace(list.toArray(new StackTraceElement[list.size()])); + return throwable; + } }