This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit f311c23a97b22ce473631101737132012320cf8a Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Wed Oct 6 16:10:58 2021 +0200 Avoid a never-ending loop in `equals(Object)` and `hashCode()` if their is recursive associations between features. --- .../org/apache/sis/feature/AbstractFeature.java | 66 ++++++++++++++++++---- .../java/org/apache/sis/feature/DenseFeature.java | 12 +++- .../java/org/apache/sis/feature/SparseFeature.java | 40 ++++++++----- 3 files changed, 90 insertions(+), 28 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java b/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java index b7754e8..dcc2fd6 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java +++ b/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java @@ -20,6 +20,7 @@ import java.util.Objects; import java.util.Iterator; import java.util.Collection; import java.util.Collections; +import java.util.IdentityHashMap; import java.io.Serializable; import org.opengis.util.ScopedName; import org.opengis.util.GenericName; @@ -78,7 +79,7 @@ import org.opengis.feature.Operation; * @author Travis L. Pinney * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.2 * * @see DefaultFeatureType#newInstance() * @@ -811,6 +812,41 @@ public abstract class AbstractFeature implements Feature, Serializable { } /** + * The features for which a {@link #hashCode()} or {@link #equals(Object)} execution are in progress. + * This is used for avoiding never-ending loop in case of recursive dependency. + */ + private static final ThreadLocal<IdentityHashMap<AbstractFeature,Boolean>> COMPARING = ThreadLocal.withInitial(IdentityHashMap::new); + + /** + * Notifies that a {@link #hashCode()} or {@link #equals(Object)} method started execution + * for the given feature and returns {@code true} if there is no recursion. + * This method must be invoked in a {@code try ... finally} block as below: + * + * {@preformat java + * if (comparisonStart()) try { + * // Compare or compute hash code. + * } finally { + * comparisonEnd(); + * } + * } + * + * @return {@code true} if hash code or equality comparison can proceed, or + * {@code false} if a recursivity is detected. + */ + final boolean comparisonStart() { + return COMPARING.get().put(this, Boolean.TRUE) == null; + } + + /** + * Notifies that the comparison of {@code this} feature is finished. + */ + final void comparisonEnd() { + if (COMPARING.get().remove(this) != Boolean.TRUE) { + throw new AssertionError(); // Should never happen. + } + } + + /** * Returns a hash code value for this feature. * The default implementation performs the following algorithm: * @@ -834,14 +870,18 @@ public abstract class AbstractFeature implements Feature, Serializable { @Override public int hashCode() { int code = type.hashCode() * 37; - for (final PropertyType pt : type.getProperties(true)) { - final String name = pt.getName().toString(); - if (name != null) { // Paranoiac check. - final Object value = getPropertyValue(name); - if (value != null) { - code += name.hashCode() ^ value.hashCode(); + if (comparisonStart()) try { + for (final PropertyType pt : type.getProperties(true)) { + final String name = pt.getName().toString(); + if (name != null) { // Paranoiac check. + final Object value = getPropertyValue(name); + if (value != null) { + code += name.hashCode() ^ value.hashCode(); + } } } + } finally { + comparisonEnd(); } return code; } @@ -878,11 +918,15 @@ public abstract class AbstractFeature implements Feature, Serializable { if (!type.equals(that.type)) { return false; } - for (final PropertyType pt : type.getProperties(true)) { - final String name = pt.getName().toString(); - if (!Objects.equals(getPropertyValue(name), that.getPropertyValue(name))) { - return false; + if (comparisonStart()) try { + for (final PropertyType pt : type.getProperties(true)) { + final String name = pt.getName().toString(); + if (!Objects.equals(getPropertyValue(name), that.getPropertyValue(name))) { + return false; + } } + } finally { + comparisonEnd(); } } return true; diff --git a/core/sis-feature/src/main/java/org/apache/sis/feature/DenseFeature.java b/core/sis-feature/src/main/java/org/apache/sis/feature/DenseFeature.java index f54948a..13564b3 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/feature/DenseFeature.java +++ b/core/sis-feature/src/main/java/org/apache/sis/feature/DenseFeature.java @@ -319,7 +319,7 @@ final class DenseFeature extends AbstractFeature implements Cloneable { @Override public int hashCode() { int code = 1; - if (properties != null) { + if (properties != null && comparisonStart()) try { if (properties instanceof Property[]) { for (final Property p : (Property[]) properties) { code = 31 * code; @@ -338,6 +338,8 @@ final class DenseFeature extends AbstractFeature implements Cloneable { } else { code = Arrays.hashCode(properties); } + } finally { + comparisonEnd(); } return type.hashCode() + code; } @@ -363,7 +365,13 @@ final class DenseFeature extends AbstractFeature implements Cloneable { wrapValuesInProperties(); } } - return Arrays.equals(properties, that.properties); + if (comparisonStart()) try { + return Arrays.equals(properties, that.properties); + } finally { + comparisonEnd(); + } else { + return true; + } } } return false; diff --git a/core/sis-feature/src/main/java/org/apache/sis/feature/SparseFeature.java b/core/sis-feature/src/main/java/org/apache/sis/feature/SparseFeature.java index 1c7108e..0d3e38c 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/feature/SparseFeature.java +++ b/core/sis-feature/src/main/java/org/apache/sis/feature/SparseFeature.java @@ -41,7 +41,7 @@ import org.opengis.feature.PropertyNotFoundException; * @author Travis L. Pinney * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.2 * * @see DenseFeature * @see DefaultFeatureType @@ -388,21 +388,25 @@ final class SparseFeature extends AbstractFeature implements Cloneable { @Override public int hashCode() { int code = type.hashCode() * 37; - if (valuesKind == PROPERTIES) { - for (final Map.Entry<Integer,Object> entry : properties.entrySet()) { - final Object p = entry.getValue(); - final Object value; - if (p instanceof Attribute<?>) { - value = getAttributeValue((Attribute<?>) p); - } else if (p instanceof FeatureAssociation) { - value = getAssociationValue((FeatureAssociation) p); - } else { - value = null; + if (comparisonStart()) try { + if (valuesKind == PROPERTIES) { + for (final Map.Entry<Integer,Object> entry : properties.entrySet()) { + final Object p = entry.getValue(); + final Object value; + if (p instanceof Attribute<?>) { + value = getAttributeValue((Attribute<?>) p); + } else if (p instanceof FeatureAssociation) { + value = getAssociationValue((FeatureAssociation) p); + } else { + value = null; + } + code += Objects.hashCode(entry.getKey()) ^ Objects.hashCode(value); } - code += Objects.hashCode(entry.getKey()) ^ Objects.hashCode(value); + } else { + code += properties.hashCode(); } - } else { - code += properties.hashCode(); + } finally { + comparisonEnd(); } return code; } @@ -428,7 +432,13 @@ final class SparseFeature extends AbstractFeature implements Cloneable { requireMapOfProperties(); } } - return properties.equals(that.properties); + if (comparisonStart()) try { + return properties.equals(that.properties); + } finally { + comparisonEnd(); + } else { + return true; + } } } return false;