BUG-7464: do not throw ISE/RTE in "impossible" cases 54/50054/9
authorRobert Varga <rovarga@cisco.com>
Thu, 5 Jan 2017 10:24:21 +0000 (11:24 +0100)
committerRobert Varga <nite@hq.sk>
Tue, 10 Jan 2017 23:44:44 +0000 (23:44 +0000)
Rather than throwing exceptions which may get mis-interpreted
by the callers, use Guava's VerifyException to flag the invariant
being checked.

Change-Id: I8a9431f1b578f3bb93fccd87f5138d97c0fa4242
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/CNode.java
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/INode.java
third-party/triemap/src/main/java/org/opendaylight/yangtools/triemap/LNodeEntries.java

index 2b3885443032b74da154a6d999785d1d1cc79945..593a66d3c21e995c92e13f93b75fd360a9738836 100644 (file)
@@ -19,6 +19,7 @@ import static org.opendaylight.yangtools.triemap.Constants.HASH_BITS;
 import static org.opendaylight.yangtools.triemap.Constants.LEVEL_BITS;
 
 import com.google.common.base.Verify;
+import com.google.common.base.VerifyException;
 import java.util.concurrent.ThreadLocalRandom;
 
 final class CNode<K, V> extends MainNode<K, V> {
@@ -74,6 +75,10 @@ final class CNode<K, V> extends MainNode<K, V> {
         return (sz = csize) != NO_SIZE ? sz : (csize = computeSize(ct));
     }
 
+    static VerifyException invalidElement(final BasicNode elem) {
+        throw new VerifyException("A CNode can contain only CNodes and SNodes, not " + elem);
+    }
+
     // lends itself towards being parallelizable by choosing
     // a random starting offset in the array
     // => if there are concurrent size computations, they start
@@ -105,7 +110,7 @@ final class CNode<K, V> extends MainNode<K, V> {
         } else if (elem instanceof INode) {
             return ((INode<?, ?>) elem).size(ct);
         } else {
-            throw new IllegalStateException("Unhandled element " + elem);
+            throw invalidElement(elem);
         }
     }
 
index e8584cf087a56e534fa7e2a630e48c88330af72f..f779868dc1b531513838be4c72c1a49bbae927f5 100644 (file)
@@ -20,6 +20,7 @@ import static org.opendaylight.yangtools.triemap.LookupResult.RESTART;
 import static org.opendaylight.yangtools.triemap.PresencePredicate.ABSENT;
 import static org.opendaylight.yangtools.triemap.PresencePredicate.PRESENT;
 
+import com.google.common.base.VerifyException;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
@@ -168,12 +169,14 @@ final class INode<K, V> extends BasicNode {
                         final CNode<K, V> rn = (cn.gen == gen) ? cn : cn.renewed(gen, ct);
                         final MainNode<K, V> nn = rn.updatedAt(pos, inode(CNode.dual(sn, k, v, hc, lev + LEVEL_BITS, gen)), gen);
                         return GCAS (cn, nn, ct);
+                    } else {
+                        throw CNode.invalidElement(cnAtPos);
                     }
-                } else {
-                    final CNode<K, V> rn = (cn.gen == gen) ? cn : cn.renewed(gen, ct);
-                    final MainNode<K, V> ncnode = rn.insertedAt(pos, flag, new SNode<>(k, v, hc), gen);
-                    return GCAS (cn, ncnode, ct);
                 }
+
+                final CNode<K, V> rn = (cn.gen == gen) ? cn : cn.renewed(gen, ct);
+                final MainNode<K, V> ncnode = rn.insertedAt(pos, flag, new SNode<>(k, v, hc), gen);
+                return GCAS (cn, ncnode, ct);
             } else if (m instanceof TNode) {
                 clean(parent, ct, lev - LEVEL_BITS);
                 return false;
@@ -182,13 +185,15 @@ final class INode<K, V> extends BasicNode {
                 final LNodeEntry<K, V> entry = ln.get(ct.equiv(), k);
                 return entry != null ? replaceln(ln, entry, v, ct) : insertln(ln, k, v, ct);
             } else {
-                throw new IllegalStateException("Unhandled node " + m);
+                throw invalidElement(m);
             }
-
-            throw new RuntimeException ("Should not happen");
         }
     }
 
+    private static VerifyException invalidElement(final BasicNode elem) {
+        throw new VerifyException("An INode can host only a CNode, a TNode or an LNode, not " + elem);
+    }
+
     /**
      * Inserts a new key value pair, given that a specific condition is met.
      *
@@ -283,6 +288,8 @@ final class INode<K, V> extends BasicNode {
 
                             return Optional.empty();
                         }
+                    } else {
+                        throw CNode.invalidElement(cnAtPos);
                     }
                 } else if (cond == null || cond == ABSENT) {
                     final CNode<K, V> rn = (cn.gen == gen) ? cn : cn.renewed(gen, ct);
@@ -329,10 +336,8 @@ final class INode<K, V> extends BasicNode {
                     return replaceln(ln, entry, v, ct) ? Optional.of(entry.getValue()) : null;
                 }
             } else {
-                throw new IllegalStateException("Unhandled node " + m);
+                throw invalidElement(m);
             }
-
-            throw new RuntimeException("Should never happen");
         }
     }
 
@@ -394,6 +399,8 @@ final class INode<K, V> extends BasicNode {
                     }
 
                     return null;
+                } else {
+                    throw CNode.invalidElement(sub);
                 }
             } else if (m instanceof TNode) {
                 // 3) non-live node
@@ -403,10 +410,8 @@ final class INode<K, V> extends BasicNode {
                 final LNodeEntry<K, V> entry = ((LNode<K, V>) m).get(ct.equiv(), k);
                 return entry != null ? entry.getValue() : null;
             } else {
-                throw new IllegalStateException("Unhandled node " + m);
+                throw invalidElement(m);
             }
-
-            throw new RuntimeException ("Should not happen");
         }
     }
 
@@ -456,19 +461,18 @@ final class INode<K, V> extends BasicNode {
 
             final int pos = Integer.bitCount(bmp & (flag - 1));
             final BasicNode sub = cn.array[pos];
-            Optional<V> res = null;
+            final Optional<V> res;
             if (sub instanceof INode) {
                 final INode<K, V> in = (INode<K, V>) sub;
                 if (startgen == in.gen) {
                     res = in.rec_remove(k, cond, hc, lev + LEVEL_BITS, this, startgen, ct);
                 } else {
-                    if (GCAS(cn, cn.renewed (startgen, ct), ct)) {
+                    if (GCAS(cn, cn.renewed(startgen, ct), ct)) {
                         res = rec_remove(k, cond, hc, lev, parent, startgen, ct);
                     } else {
                         res = null;
                     }
                 }
-
             } else if (sub instanceof SNode) {
                 final SNode<K, V> sn = (SNode<K, V>) sub;
                 if (sn.hc == hc && ct.equal(sn.k, k) && (cond == null || cond.equals(sn.v))) {
@@ -481,6 +485,8 @@ final class INode<K, V> extends BasicNode {
                 } else {
                     res = Optional.empty();
                 }
+            } else {
+                throw CNode.invalidElement(sub);
             }
 
             if (res == null || !res.isPresent()) {
@@ -515,7 +521,7 @@ final class INode<K, V> extends BasicNode {
 
             return GCAS(ln, ln.removeChild(entry, hc), ct) ? Optional.of(value) : null;
         } else {
-            throw new IllegalStateException("Unhandled node " + m);
+            throw invalidElement(m);
         }
     }
 
index 2fbfc7299575c3bd7646737431a4c83abc585734..66cb8186400ff5ce4a4a4d9c39d2d77a76f0c6a8 100644 (file)
@@ -15,6 +15,8 @@
  */
 package org.opendaylight.yangtools.triemap;
 
+import com.google.common.base.VerifyException;
+
 /**
  * Similar to Scala's ListMap, this is a single-linked list of set of map entries. Aside from the Set contract, this
  * class fulfills the requirements for an immutable map entryset.
@@ -122,6 +124,6 @@ abstract class LNodeEntries<K, V> extends LNodeEntry<K, V> {
             cur = cur.next();
         }
 
-        throw new IllegalStateException(String.format("Entry %s not found", entry));
+        throw new VerifyException(String.format("Failed to find entry %s", entry));
     }
 }