Issue Details (XML | Word | Printable)

Key: GROOVY-1904
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Jochen Theodorou
Reporter: Jörg Staudemeyer
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
groovy

Iterators do not close streams

Created: 21/May/07 09:33 AM   Updated: 01/Jul/07 02:16 PM
Component/s: groovy-jdk
Affects Version/s: 1.0, 1.1-beta-1
Fix Version/s: 1.1-beta-2

Time Tracking:
Not Specified

File Attachments: 1. Text File DefaultGroovyMethods-Patch.txt (6 kB)
2. File DefaultGroovyMethodsFileIteratorTest.groovy (4 kB)
3. File DefaultGroovyMethodsFileIteratorTest.groovy (5 kB)


Testcase included: yes


 Description  « Hide
iterator() methods for File, Reader, InputStream, and DataInputStream return iterators that do not close their underlying streams. They should do this, to be consistent with iterative closure methods like eachLine() etc. A solution must take in account that a loop can be broken by an exception inside the loop, that is not caused by a resource read operation. Also in this case the resource must be closed.

See discussion at http://www.nabble.com/File-Iterator-leaves-Reader-open-tf3782079.html.

To clarify problem a JUnit test case will is attached.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Jörg Staudemeyer added a comment - 22/May/07 05:01 AM
According to the results of the mentioned discussion following modifications shall be applied to DefaultGroovyMethods:
  • New overloaded each() methods on File, Reader, InputStream, and ObjectInputStream close their resources on EOF and when an exception occurs. In some cases they may simple delegate to specialized each-methods (like eachLine()).
  • The iterator() methods on Reader, InputStream, and ObjectInputStream return iterators that close their underlying resource on EOF or IOException.
  • The iterator() method on File throws an groovy.lang.DeprecationException stating "Iterators on files are not supported any more. Use File.eachLine() instead. Alternatively you can use FileReader.iterator() and provide your own exception handling."

This results in the following consequences for application programmers:

  • each() loops on streams and readers can be used safely, streams and readers will be closed, no matter what happens inside the loops.
  • For-loops on streams and readers close their streams and readers on normal execution but not necessarily when an exception occurs. So such loops must always be enclosed in try blocks with a finally block that closes the resource.
  • For-loops on files do not work anymore, since there is no way to handle exceptions. They can be easily replaced by File.each() or - more expressive - by File.eachLine().

All test methods that relate to handling of exceptions inside the loop can be removed from the attached JUnit file.


Jörg Staudemeyer added a comment - 24/May/07 10:07 AM
Attached patch file for DefaultGroovyMethods. Further directives are written directly to the file.

Attached also an updated unit testcase reflecting the actual requirements.


Jochen Theodorou added a comment - 01/Jul/07 02:16 PM
I close this issue now because I have deprecated File#iterator.

The solution with the Iterator was discussed, but not approved... it is a pattern too hidden from the user - it is difficult to know then if it will close or not.

I will also not change/add the "each" Methods on File, Reader and InputStream... One good example here is DataInputStream where people usually expect mixed, not only raw bytes. And using eachLine as "each" for File... We already have eachLine, eachObject and eachByte and they fit exactly the needs here.. I think it is not of much help to provide here an alias to existing methods that might do what they expect for one, but not for the other.

That means Object#each will be used which tries not to make them into arrays, lists or other collections and just iterators over a one element list containing the object we operate on. That might be not of much use, but it is also not harmful