GeoTools

Duplicate FeatureReaderIterator class leads to unclosed iterators

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Not A Bug
  • Affects Version/s: 2.5.1
  • Fix Version/s: 2.5.2, 2.6-M0
  • Component/s: main
  • Labels:
    None

Description

There are two FeatureReaderIterator classes:

  • org.geotools.data.store.FeatureReaderIterator
  • org.geotools.feature.FeatureReaderIterator

both are identical in purpose and almost identical in code.

The first being the one DataFeatureCollection.closeIterator( Iterator<SimpleFeature> close ) does an instanceof check to call the iterator's close() method, but is not used as far as I can see by looking for references to it.

The second being the only one referenced, used by the arcsde and wfs modules. But close() being ignored since DataFeatureCollection does not check for it.

The solution would be to deprecate the one in the org.geotools.feature package and make it an empty class extending from the one in org.geotools.data.store for 2.5.x and just remove it on trunk.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Gabriel Roldan added a comment -
hum.. problem being {{org.geotools.data.store.FeatureReaderIterator}} is package protected and final...
Jody would you mind if we make it public and non final? (ie, for arcsde I need it cause I have my own extension of DataFeatureCollection)
Show
Gabriel Roldan added a comment - hum.. problem being {{org.geotools.data.store.FeatureReaderIterator}} is package protected and final... Jody would you mind if we make it public and non final? (ie, for arcsde I need it cause I have my own extension of DataFeatureCollection)
Hide
Permalink
Gabriel Roldan added a comment -
that is, DataFeatureCollection is abstract so designed for extension, and I need to extend both DataFeatureCollection and the iterators it creates for a matter of arcsde connection handling. It makes sense to me to make FeatureReaderIterator public and non final in order to leverage the extensibility of DataFeatureCollection.
Show
Gabriel Roldan added a comment - that is, DataFeatureCollection is abstract so designed for extension, and I need to extend both DataFeatureCollection and the iterators it creates for a matter of arcsde connection handling. It makes sense to me to make FeatureReaderIterator public and non final in order to leverage the extensibility of DataFeatureCollection.
Hide
Permalink
Gabriel Roldan added a comment -
And btw, I got to this issue while debugging udig and found my wfs iterators were not being closed when the map's extent changes while a layer did not finish rendering
Show
Gabriel Roldan added a comment - And btw, I got to this issue while debugging udig and found my wfs iterators were not being closed when the map's extent changes while a layer did not finish rendering
Hide
Permalink
Gabriel Roldan added a comment -
hum... investigating this whole thing I realized the intended way to use DataFeatureCollection is simply to override reader():FeatureReader so it takes care of wrapping it with FeatureReaderIterator and hence any extra logic I may need should go in the returned FeatureReader instead of subclassing FeatureReaderIterator... and hence it makes sense to keep it non public.
So you can disregard the petition of making FeatureReaderIterator public but there's still the other one. Andrea already provided a fix for the good one (the one DataFeatureCollection uses) so it may still be a good thing to keep just one around?
Show
Gabriel Roldan added a comment - hum... investigating this whole thing I realized the intended way to use DataFeatureCollection is simply to override reader():FeatureReader so it takes care of wrapping it with FeatureReaderIterator and hence any extra logic I may need should go in the returned FeatureReader instead of subclassing FeatureReaderIterator... and hence it makes sense to keep it non public. So you can disregard the petition of making FeatureReaderIterator public but there's still the other one. Andrea already provided a fix for the good one (the one DataFeatureCollection uses) so it may still be a good thing to keep just one around?
Hide
Permalink
Gabriel Roldan added a comment -
since it turned out I was making bad use of the DataFeatureCollection "framework" I'm closing it as "Not a bug".
Whether to let a single FeatureReaderIterator around seems like a separate discussion
Show
Gabriel Roldan added a comment - since it turned out I was making bad use of the DataFeatureCollection "framework" I'm closing it as "Not a bug". Whether to let a single FeatureReaderIterator around seems like a separate discussion

People

  • Assignee:
    Gabriel Roldan
    Reporter:
    Gabriel Roldan
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: