This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 21eccc4ad07 IGNITE-28623 SQL Calcite: Fix premature orphan iterator
closing by GC - Fixes #13089.
21eccc4ad07 is described below
commit 21eccc4ad073a0675738a413394e015f3cd96572
Author: Aleksey Plekhanov <[email protected]>
AuthorDate: Wed May 6 12:41:49 2026 +0300
IGNITE-28623 SQL Calcite: Fix premature orphan iterator closing by GC -
Fixes #13089.
Signed-off-by: Aleksey Plekhanov <[email protected]>
---
.../calcite/exec/ClosableIteratorsHolder.java | 64 ++++++++++++++++++----
.../query/calcite/exec/rel/RootNode.java | 9 +--
.../processors/query/calcite/CancelTest.java | 3 +-
3 files changed, 57 insertions(+), 19 deletions(-)
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ClosableIteratorsHolder.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ClosableIteratorsHolder.java
index caae30f12d9..e6c0b499190 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ClosableIteratorsHolder.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ClosableIteratorsHolder.java
@@ -28,6 +28,7 @@ import java.util.function.Consumer;
import org.apache.ignite.IgniteLogger;
import org.apache.ignite.internal.processors.query.calcite.util.Commons;
import org.apache.ignite.internal.util.typedef.internal.U;
+import org.jetbrains.annotations.Nullable;
/**
*/
@@ -37,7 +38,7 @@ public class ClosableIteratorsHolder {
private final ReferenceQueue refQueue;
/** */
- private final Map<Reference, Object> refMap;
+ private final Map<Reference, Iterator<?>> refMap;
/** */
private final IgniteLogger log;
@@ -97,11 +98,15 @@ public class ClosableIteratorsHolder {
}
/** */
- private AutoCloseable closeable(Object referent, Object resource) {
- if (!(resource instanceof AutoCloseable))
+ private @Nullable AutoCloseable closeable(Object referent, Iterator<?>
rsrc) {
+ if (!(rsrc instanceof AutoCloseable))
return null;
- return new CloseableReference(referent, resource);
+ CloseableReference ref = new CloseableReference(referent);
+
+ refMap.put(ref, rsrc);
+
+ return ref;
}
/** */
@@ -109,6 +114,14 @@ public class ClosableIteratorsHolder {
/** */
private final Iterator<T> delegate;
+ /**
+ * This variable is required to keep reference to current instance
while delegate call
+ * (hasNext/next/remove/forEachRemaining) is not completed. We
actually don't care about variable value and
+ * thread safety, it's only to prevent premature garbage collection
and iterator closing before returning
+ * result to the user.
+ */
+ private boolean inUse;
+
/** */
private final AutoCloseable closeable;
@@ -119,26 +132,57 @@ public class ClosableIteratorsHolder {
/** {@inheritDoc} */
@Override public boolean hasNext() {
- return delegate.hasNext();
+ inUse = true;
+
+ try {
+ return delegate.hasNext();
+ }
+ finally {
+ inUse = false;
+ }
}
/** {@inheritDoc} */
@Override public T next() {
- return delegate.next();
+ inUse = true;
+
+ try {
+ return delegate.next();
+ }
+ finally {
+ inUse = false;
+ }
}
/** {@inheritDoc} */
@Override public void remove() {
- delegate.remove();
+ inUse = true;
+
+ try {
+ delegate.remove();
+ }
+ finally {
+ inUse = false;
+ }
}
/** {@inheritDoc} */
@Override public void forEachRemaining(Consumer<? super T> action) {
- delegate.forEachRemaining(action);
+ inUse = true;
+
+ try {
+ delegate.forEachRemaining(action);
+ }
+ finally {
+ inUse = false;
+ }
}
/** {@inheritDoc} */
@Override public void close() throws Exception {
+ if (log.isDebugEnabled())
+ log.debug("Closing iterator [delegate=" + delegate + ",
inUse=" + inUse + ']');
+
Commons.close(closeable);
}
}
@@ -146,10 +190,8 @@ public class ClosableIteratorsHolder {
/** */
private final class CloseableReference extends WeakReference implements
AutoCloseable {
/** */
- private CloseableReference(Object referent, Object resource) {
+ private CloseableReference(Object referent) {
super(referent, refQueue);
-
- refMap.put(this, resource);
}
/** {@inheritDoc} */
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/RootNode.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/RootNode.java
index 891cff97c79..2c401bcd592 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/RootNode.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/RootNode.java
@@ -28,7 +28,6 @@ import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
import java.util.function.Function;
-
import org.apache.calcite.rel.type.RelDataType;
import org.apache.ignite.IgniteInterruptedException;
import org.apache.ignite.cache.query.QueryCancelledException;
@@ -205,8 +204,6 @@ public class RootNode<Row> extends AbstractNode<Row>
implements SingleNode<Row>,
/** {@inheritDoc} */
@Override public boolean hasNext() {
- checkException();
-
if (!outBuff.isEmpty())
return true;
@@ -324,9 +321,7 @@ public class RootNode<Row> extends AbstractNode<Row>
implements SingleNode<Row>,
if (e == null)
return;
- if (e instanceof IgniteSQLException)
- throw (IgniteSQLException)e;
- else
- throw new IgniteSQLException("An error occurred while query
executing - " + e.getMessage(), IgniteQueryErrorCode.UNKNOWN, e);
+ throw new IgniteSQLException("An error occurred while query executing
- " + e.getMessage(),
+ e instanceof IgniteSQLException ?
((IgniteSQLException)e).statusCode() : IgniteQueryErrorCode.UNKNOWN, e);
}
}
diff --git
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CancelTest.java
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CancelTest.java
index 4e78cbe40fd..c57f4ff77b5 100644
---
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CancelTest.java
+++
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/CancelTest.java
@@ -102,7 +102,8 @@ public class CancelTest extends GridCommonAbstractTest {
cursors.forEach(QueryCursor::close);
GridTestUtils.assertThrows(log, () -> {
- it.next();
+ while (it.hasNext())
+ it.next();
return null;
},
