You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/09/07 05:25:19 UTC
[GitHub] [tinkerpop] rdtr opened a new pull request #1325: [TINKERPOP-2416] MultiIterator implement AutoCloseable and close hold…
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
[GitHub] [tinkerpop] rdtr commented on pull request #1325: [TINKERPOP-2416] MultiIterator should implement AutoCloseable
Posted by GitBox <gi...@apache.org>.
rdtr commented on pull request #1325:
URL: https://github.com/apache/tinkerpop/pull/1325#issuecomment-689428667
I agree that we don't need to explicitly call `close` in TinkerPop's code.
The change is mainly for providers. When overriding TinkerPop's API to retrieve elements from their own internal Storage layer, if the returned iterator holds any reference which must be released in the end, it is the provider's responsibility to call `close` in their own method.
TinkerPop's base class and TinkerGraph doesn't have the resource issue so we don't need to call `close` there.
----------------------------------------------------------------
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
[GitHub] [tinkerpop] spmallette commented on pull request #1325: [TINKERPOP-2416] MultiIterator should implement AutoCloseable
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1325:
URL: https://github.com/apache/tinkerpop/pull/1325#issuecomment-688785640
There are a number of places in TinkerPop code that handle `MultiIterator` instances. I guess we don't need to explicitly `close()` those as I don't think they hold expensive resources open (that I am aware of). Any thoughts on that?
----------------------------------------------------------------
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
[GitHub] [tinkerpop] spmallette merged pull request #1325: [TINKERPOP-2416] MultiIterator should implement AutoCloseable
Posted by GitBox <gi...@apache.org>.
spmallette merged pull request #1325:
URL: https://github.com/apache/tinkerpop/pull/1325
----------------------------------------------------------------
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
[GitHub] [tinkerpop] spmallette commented on pull request #1325: [TINKERPOP-2416] MultiIterator should implement AutoCloseable
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1325:
URL: https://github.com/apache/tinkerpop/pull/1325#issuecomment-689472760
VOTE +1
----------------------------------------------------------------
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