You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2021/12/21 13:39:09 UTC
[tinkerpop] branch TINKERPOP-2626 created (now d31cd6c)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2626
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at d31cd6c TINKERPOP-2626 Added explicit closed state to DefaultTraversal
This branch includes the following new commits:
new d31cd6c TINKERPOP-2626 Added explicit closed state to DefaultTraversal
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2626 Added explicit closed state to DefaultTraversal
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2626
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit d31cd6cf251810ccca2643d9a97878b642cef91b
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Tue Dec 21 08:37:18 2021 -0500
TINKERPOP-2626 Added explicit closed state to DefaultTraversal
The closed state prevents futher iteration on a traversal when it has been exhausted and resources have been released. This change is non-breaking but should likely be refactored when merged to branch that can take a breaking change.
---
CHANGELOG.asciidoc | 1 +
.../gremlin/process/traversal/Traversal.java | 14 +++++++-
.../process/traversal/util/DefaultTraversal.java | 38 ++++++++++++++++++++--
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index ed87ae9..f2df59e 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -24,6 +24,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
=== TinkerPop 3.4.13 (Release Date: NOT OFFICIALLY RELEASED YET)
* Fixed `RangeGlobalStep` which was prematurely closing the iterator.
+* Added explicit state to `DefaultTraversal` to track whether or not it was fully iterated and closed to ensure it released resources properly.
* Prevented XML External Entity (XXE) style attacks via `GraphMLReader` by disabling DTD and external entities by default.
* Improved error message for failed serialization for HTTP-based requests.
* Fixed a `NullPointerException` that could occur during a failed `Connection` initialization due to uninstantiated `AtomicInteger`.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
index b1b45f3..0284fff 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -285,7 +285,8 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
}
/**
- * Releases resources opened in any steps that implement {@link AutoCloseable}.
+ * Releases resources opened in any steps that implement {@link AutoCloseable}. If this method is overridden,the
+ * implementer should invoke {@link #notifyClose()}.
*/
@Override
public default void close() throws Exception {
@@ -293,6 +294,17 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
if (step instanceof AutoCloseable)
((AutoCloseable) step).close();
}
+
+ notifyClose();
+ }
+
+ /**
+ * Gets a callback from {@link #close()} for additional operations specific to the {@link Traversal} implementation.
+ * A good implementation will use {@link #close()} to release resources in steps and this method to release
+ * resources specific to the {@link Traversal} implementations.
+ */
+ public default void notifyClose() {
+ // do nothing by default
}
/**
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
index 3aa7bc1..83a06f7 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
@@ -65,6 +65,7 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
protected Set<TraverserRequirement> requirements;
protected boolean locked = false;
+ protected boolean closed = false;
protected Bytecode bytecode;
private DefaultTraversal(final Graph graph, final TraversalStrategies traversalStrategies, final Bytecode bytecode) {
@@ -192,6 +193,9 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
@Override
public boolean hasNext() {
+ // if the traversal is closed then resources are released and there is nothing else to iterate
+ if (this.isClosed()) return false;
+
if (!this.locked) this.applyStrategies();
final boolean more = this.lastTraverser.bulk() > 0L || this.finalEndStep.hasNext();
if (!more) CloseableIterator.closeIterator(this);
@@ -200,6 +204,10 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
@Override
public E next() {
+ // if the traversal is closed then resources are released and there is nothing else to iterate
+ if (this.isClosed())
+ throw this.parent instanceof EmptyStep ? new NoSuchElementException() : FastNoSuchElementException.instance();
+
try {
if (!this.locked) this.applyStrategies();
if (this.lastTraverser.bulk() == 0L)
@@ -220,18 +228,32 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
this.steps.forEach(Step::reset);
this.finalEndStep.reset();
this.lastTraverser = EmptyTraverser.instance();
+ this.closed = false;
}
@Override
public void addStart(final Traverser.Admin<S> start) {
if (!this.locked) this.applyStrategies();
- if (!this.steps.isEmpty()) this.steps.get(0).addStart(start);
+ if (!this.steps.isEmpty()) {
+ this.steps.get(0).addStart(start);
+
+ // match() expects that a traversal that has been iterated can continue to iterate if new starts are
+ // added therefore the closed state must be reset.
+ this.closed = false;
+ }
}
@Override
public void addStarts(final Iterator<Traverser.Admin<S>> starts) {
if (!this.locked) this.applyStrategies();
- if (!this.steps.isEmpty()) this.steps.get(0).addStarts(starts);
+
+ if (!this.steps.isEmpty()) {
+ this.steps.get(0).addStarts(starts);
+
+ // match() expects that a traversal that has been iterated can continue to iterate if new starts are
+ // added therefore the closed state must be reset.
+ this.closed = false;
+ }
}
@Override
@@ -279,6 +301,18 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
return this.locked;
}
+ /**
+ * Determines if the traversal has been fully iterated and resources released.
+ */
+ public boolean isClosed() {
+ return closed;
+ }
+
+ @Override
+ public void notifyClose() {
+ this.closed = true;
+ }
+
@Override
public void setSideEffects(final TraversalSideEffects sideEffects) {
this.sideEffects = sideEffects;