You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2021/10/06 16:27:35 UTC
[sis] 01/02: Avoid a never-ending loop in `equals(Object)` and
`hashCode()` if their is recursive associations between features.
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 <ma...@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;