[jira] [Commented] (TINKERPOP-2416) MultiIterator should implement AutoCloseable

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TINKERPOP-2416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17191443#comment-17191443 ] 

ASF GitHub Bot commented on TINKERPOP-2416:
-------------------------------------------

rdtr opened a new pull request #1325:
URL: https://github.com/apache/tinkerpop/pull/1325


   As described in the [JIRA](https://issues.apache.org/jira/browse/TINKERPOP-2416), currently `MultiIterator` does not implement `AutoCloseable`. This potentially introduces resource leak when we have multiple iterators that implement `AutoCloseable` and combine them using `IteratorUtils#concat`. The caller may expect all iterators to be closed automatically by calling `CloseableIterator#closeIterator` but right now `MultiIterator` doesn't implement AutoCloseable so even the underlying iterators are auto-closeable, they are never be closed.
   
   This simple PR just adds AutoCloseable implementation to MultiIterator, close all inner iterators if they implement AutoCloseable.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> MultiIterator should implement AutoCloseable
> --------------------------------------------
>
>                 Key: TINKERPOP-2416
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2416
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: structure
>    Affects Versions: 3.4.8
>            Reporter: Norio Akagi
>            Priority: Minor
>
> TinkerPop has a handy set of util methods for Iterators and specifically IteratorUtils#concat provides a way to combine multiple iterators and treat it as if we have a single iterator.
> [https://github.com/apache/tinkerpop/blob/master/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/IteratorUtils.java#L352]
> While this is convenient, when we use it we had some memory leak issue.
> The reason is because sometimes an iterator we use needs to be explicitly closed when it holds underlying resource to be released. TinkerPop is aware of this pattern and it has [CloseableIterator#closeIterator |[https://github.com/apache/tinkerpop/blob/cff4c161615f2b50bda27b6ba523c7f52b833532/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java#L50]]method that closes an iterator if it implements AutoCloseable.
> However MultiIterator which Tinkerpop uses to combine multiple iterators when concat method is called does not implement AutoCloseable therefore when we use concat method while expecting the iterator is eventually closed by CloseableIterator#closeIterator method, it leaves the underlying iterators unclosed then it causes some resources never released.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)