From 2543a8f1a9b6f062636ee566a1955e0fb1befdca Mon Sep 17 00:00:00 2001 From: nickreid Date: Mon, 25 Jun 2018 12:53:26 -0700 Subject: [PATCH] Restructures `hashCode()` on `JSType` and sub-types to prevent possible infinite recursions. This change also centralizes some logic and vague comments about nominal type hashcodes. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=202002954 --- .../javascript/rhino/jstype/AllType.java | 2 +- .../javascript/rhino/jstype/ArrowType.java | 2 +- .../rhino/jstype/EnumElementType.java | 10 ++---- .../javascript/rhino/jstype/FunctionType.java | 2 +- .../rhino/jstype/InstanceObjectType.java | 8 ++--- .../javascript/rhino/jstype/JSType.java | 36 +++++++++++++++++-- .../javascript/rhino/jstype/NamedType.java | 16 ++++++--- .../javascript/rhino/jstype/NoObjectType.java | 2 +- .../rhino/jstype/PrototypeObjectType.java | 2 +- .../rhino/jstype/ProxyObjectType.java | 2 +- .../rhino/jstype/TemplatizedType.java | 4 +-- .../javascript/rhino/jstype/UnionType.java | 2 +- .../javascript/rhino/jstype/UnknownType.java | 2 +- .../javascript/rhino/jstype/ValueType.java | 3 +- 14 files changed, 60 insertions(+), 33 deletions(-) diff --git a/src/com/google/javascript/rhino/jstype/AllType.java b/src/com/google/javascript/rhino/jstype/AllType.java index b353fa0906c..abca40b6817 100644 --- a/src/com/google/javascript/rhino/jstype/AllType.java +++ b/src/com/google/javascript/rhino/jstype/AllType.java @@ -110,7 +110,7 @@ JSType resolveInternal(ErrorReporter reporter) { } @Override - public int hashCode() { + int recursionUnsafeHashCode() { return System.identityHashCode(this); } diff --git a/src/com/google/javascript/rhino/jstype/ArrowType.java b/src/com/google/javascript/rhino/jstype/ArrowType.java index 271440bf0fa..80eefd415b4 100644 --- a/src/com/google/javascript/rhino/jstype/ArrowType.java +++ b/src/com/google/javascript/rhino/jstype/ArrowType.java @@ -220,7 +220,7 @@ boolean checkArrowEquivalenceHelper( } @Override - public int hashCode() { + int recursionUnsafeHashCode() { int hashCode = Objects.hashCode(returnType); if (parameters != null) { Node param = parameters.getFirstChild(); diff --git a/src/com/google/javascript/rhino/jstype/EnumElementType.java b/src/com/google/javascript/rhino/jstype/EnumElementType.java index 46028302bda..c8d71088007 100644 --- a/src/com/google/javascript/rhino/jstype/EnumElementType.java +++ b/src/com/google/javascript/rhino/jstype/EnumElementType.java @@ -39,7 +39,6 @@ package com.google.javascript.rhino.jstype; -import static com.google.common.base.Preconditions.checkState; import com.google.javascript.rhino.ErrorReporter; import com.google.javascript.rhino.Node; @@ -142,14 +141,9 @@ public boolean isNominalType() { return hasReferenceName(); } - /** - * If this is equal to a NamedType object, its hashCode must be equal - * to the hashCode of the NamedType object. - */ @Override - public int hashCode() { - checkState(hasReferenceName()); - return getReferenceName().hashCode(); + int recursionUnsafeHashCode() { + return NamedType.nominalHashCode(this); } @Override diff --git a/src/com/google/javascript/rhino/jstype/FunctionType.java b/src/com/google/javascript/rhino/jstype/FunctionType.java index 8d4d55a2fd3..3b190b857b2 100644 --- a/src/com/google/javascript/rhino/jstype/FunctionType.java +++ b/src/com/google/javascript/rhino/jstype/FunctionType.java @@ -919,7 +919,7 @@ final boolean checkFunctionEquivalenceHelper( } @Override - public int hashCode() { + int recursionUnsafeHashCode() { int hc = kind.hashCode(); switch (kind) { case CONSTRUCTOR: diff --git a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java index 7422f4ad3a2..a1f0936bf72 100644 --- a/src/com/google/javascript/rhino/jstype/InstanceObjectType.java +++ b/src/com/google/javascript/rhino/jstype/InstanceObjectType.java @@ -167,14 +167,10 @@ public boolean isNominalType() { return hasReferenceName(); } - /** - * If this is equal to a NamedType object, its hashCode must be equal - * to the hashCode of the NamedType object. - */ @Override - public int hashCode() { + int recursionUnsafeHashCode() { if (hasReferenceName()) { - return getReferenceName().hashCode(); + return NamedType.nominalHashCode(this); } else { return super.hashCode(); } diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java index 725aa7bcd1f..274681259a3 100644 --- a/src/com/google/javascript/rhino/jstype/JSType.java +++ b/src/com/google/javascript/rhino/jstype/JSType.java @@ -76,6 +76,8 @@ public abstract class JSType implements Serializable { private JSType resolveResult = null; protected TemplateTypeMap templateTypeMap; + private boolean hashCodeInProgress = false; + private boolean inTemplatedCheckVisit = false; private static final CanCastToVisitor CAN_CAST_TO_VISITOR = new CanCastToVisitor(); @@ -765,12 +767,40 @@ public boolean equals(Object jsType) { return (jsType instanceof JSType) && isEquivalentTo((JSType) jsType); } + /** + * Calculates a hash of the object as per {@link Object#hashCode()}. + * + *

This method is unsafe for multi-threaded use. The implementation mutates instance + * state to prevent recursion and therefore expects sole access. + */ @Override - public abstract int hashCode(); + public final int hashCode() { + if (hashCodeInProgress) { + return -1; // Recursive base-case. + } + + this.hashCodeInProgress = true; + int hashCode = recursionUnsafeHashCode(); + this.hashCodeInProgress = false; + return hashCode; + } /** - * This predicate is used to test whether a given type can appear in a - * numeric context, such as an operand of a multiply operator. + * Calculates {@code #hashCode()} with the assumption that it will never be called recursively. + * + *

To work correctly this method should only called under the following conditions: + * + *

+ */ + abstract int recursionUnsafeHashCode(); + + /** + * This predicate is used to test whether a given type can appear in a numeric context, such as an + * operand of a multiply operator. */ public boolean matchesNumberContext() { return false; diff --git a/src/com/google/javascript/rhino/jstype/NamedType.java b/src/com/google/javascript/rhino/jstype/NamedType.java index ec9ab3a4f00..d3483d3af3e 100644 --- a/src/com/google/javascript/rhino/jstype/NamedType.java +++ b/src/com/google/javascript/rhino/jstype/NamedType.java @@ -40,6 +40,8 @@ package com.google.javascript.rhino.jstype; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.base.Strings.emptyToNull; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; @@ -84,6 +86,12 @@ public final class NamedType extends ProxyObjectType { private static final long serialVersionUID = 1L; + static int nominalHashCode(ObjectType type) { + checkState(type.hasReferenceName()); + String name = checkNotNull(emptyToNull(type.getReferenceName())); + return name.hashCode(); + } + private final String reference; private final String sourceName; private final int lineno; @@ -209,13 +217,11 @@ public boolean isNominalType() { } @Override - public int hashCode() { - return reference.hashCode(); + int recursionUnsafeHashCode() { + return nominalHashCode(this); } - /** - * Resolve the referenced type within the enclosing scope. - */ + /** Resolve the referenced type within the enclosing scope. */ @Override JSType resolveInternal(ErrorReporter reporter) { diff --git a/src/com/google/javascript/rhino/jstype/NoObjectType.java b/src/com/google/javascript/rhino/jstype/NoObjectType.java index a17d069d5d0..b5d03c68ecb 100644 --- a/src/com/google/javascript/rhino/jstype/NoObjectType.java +++ b/src/com/google/javascript/rhino/jstype/NoObjectType.java @@ -144,7 +144,7 @@ public final boolean matchesSymbolContext() { } @Override - public final int hashCode() { + int recursionUnsafeHashCode() { return System.identityHashCode(this); } diff --git a/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java b/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java index 853f9b1a420..7f5cefdec2e 100644 --- a/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java +++ b/src/com/google/javascript/rhino/jstype/PrototypeObjectType.java @@ -555,7 +555,7 @@ public void matchRecordTypeConstraint(ObjectType constraintObj) { } @Override - public int hashCode() { + int recursionUnsafeHashCode() { if (isStructuralType()) { return Objects.hash(className, properties); } else { diff --git a/src/com/google/javascript/rhino/jstype/ProxyObjectType.java b/src/com/google/javascript/rhino/jstype/ProxyObjectType.java index 4174da72e0c..e4bea639570 100644 --- a/src/com/google/javascript/rhino/jstype/ProxyObjectType.java +++ b/src/com/google/javascript/rhino/jstype/ProxyObjectType.java @@ -290,7 +290,7 @@ public Iterable getCtorExtendedInterfaces() { } @Override - public int hashCode() { + int recursionUnsafeHashCode() { return referencedType.hashCode(); } diff --git a/src/com/google/javascript/rhino/jstype/TemplatizedType.java b/src/com/google/javascript/rhino/jstype/TemplatizedType.java index 12e7a520104..b7922858e02 100644 --- a/src/com/google/javascript/rhino/jstype/TemplatizedType.java +++ b/src/com/google/javascript/rhino/jstype/TemplatizedType.java @@ -123,8 +123,8 @@ StringBuilder appendTo(StringBuilder sb, boolean forAnnotations) { } @Override - public int hashCode() { - int baseHash = super.hashCode(); + int recursionUnsafeHashCode() { + int baseHash = super.recursionUnsafeHashCode(); // TODO(b/110224889): This case can probably be removed if `equals()` is updated. if (isSpecializedOnlyWithUnknown) { diff --git a/src/com/google/javascript/rhino/jstype/UnionType.java b/src/com/google/javascript/rhino/jstype/UnionType.java index bc2d884b92d..34e062c3bb4 100644 --- a/src/com/google/javascript/rhino/jstype/UnionType.java +++ b/src/com/google/javascript/rhino/jstype/UnionType.java @@ -508,7 +508,7 @@ public HasPropertyKind getPropertyKind(String pname, boolean autobox) { } @Override - public int hashCode() { + int recursionUnsafeHashCode() { return this.hashcode; } diff --git a/src/com/google/javascript/rhino/jstype/UnknownType.java b/src/com/google/javascript/rhino/jstype/UnknownType.java index 6f33c9172ad..73804fcc2a9 100644 --- a/src/com/google/javascript/rhino/jstype/UnknownType.java +++ b/src/com/google/javascript/rhino/jstype/UnknownType.java @@ -176,7 +176,7 @@ JSType resolveInternal(ErrorReporter reporter) { } @Override - public int hashCode() { + int recursionUnsafeHashCode() { return System.identityHashCode(this); } } diff --git a/src/com/google/javascript/rhino/jstype/ValueType.java b/src/com/google/javascript/rhino/jstype/ValueType.java index 1080185669c..3f3f8a457dc 100644 --- a/src/com/google/javascript/rhino/jstype/ValueType.java +++ b/src/com/google/javascript/rhino/jstype/ValueType.java @@ -77,7 +77,8 @@ public HasPropertyKind getPropertyKind(String propertyName, boolean autobox) { } @Override - public final int hashCode() { + final int recursionUnsafeHashCode() { + // Subclasses of this type are unique within a JSTypeRegisty. return System.identityHashCode(this); } }