Skip to content

[Bug] Store MultiListIterator leaks its sub-iterators when closed before the first hasNext() #3436

Closed

Description

@vaidikgarg-bit

Bug Type (问题类型)

logic (逻辑设计问题)

Environment (环境信息)

  • Server Version: master at 29333a6ec (1.8.0-SNAPSHOT)
  • Backend: HStore (hg-store-core MultiListIterator)
  • JDK 17

Expected & Actual behavior (期望与实际表现)

Closing a MultiListIterator should close every sub-iterator it holds. If close() is called before the first hasNext(), none of them are closed.

close() only closes innerIterator and whatever is left in innerListIterator:

if (this.innerIterator != null) {
    this.innerIterator.close();
}
if (this.innerListIterator != null) {
    while (this.innerListIterator.hasNext()) {
        this.innerListIterator.next().close();
    }
}
this.iterators.clear();

Both fields are set lazily in getInnerIterator(), which runs on the first hasNext() or isValid() call. Before that they are null, so close() skips both blocks and then clears the list. The sub-iterators are never closed and can't be reached anymore.

BusinessHandlerImpl builds the sub-iterators up front (scan, scanPrefix, indexIntersection, BatchGetIterator), then wraps them in a MultiListIterator for multi-param scans and multi-index queries, often inside a dedup wrapper whose close() just calls through. One path that closes it before iterating is AggregativeQueryObserver: it creates the iterator, and if the client cancels before the read loop runs while (!this.clientCanceled.get() && this.iterator.hasNext()), it exits without ever calling hasNext(). The task then calls iterator.close(), and the store-side scan iterators stay open.

Reproduction (in FilterIteratorContractTest):

RowIterator first = new RowIterator(List.of(BackendColumn.of(new byte[]{1}, new byte[]{11})));
RowIterator second = new RowIterator(List.of(BackendColumn.of(new byte[]{2}, new byte[]{12})));
MultiListIterator iterator = new MultiListIterator(List.of(first, second));

iterator.close();

assertTrue(first.closed);   // fails on master
assertTrue(second.closed);

It fails the same way when wrapped in MapLimitIterator.

I'd like to work on this: when close() runs before iteration starts, close every sub-iterator in the list. Sub-iterators that were already exhausted and closed during iteration would not be closed a second time.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions