GROOVY-8373: Fix resource leak in filterLine Writable.writeTo method

[blackdrag]

The filterLine method returns a Writable whose writeTo method reads from a BufferedReader but never closes it, causing the underlying file handle to remain open and cause resource leaks.

This fix wraps the filtering logic in writeTo within a try-finally block to ensure the BufferedReader is closed after use.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.0065%. Comparing base (cc8bc3e) to head (f3b6fdd).

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2352        +/-   ##
==================================================
- Coverage     67.0080%   67.0065%   -0.0014%     
+ Complexity      29357      29355         -2     
==================================================
  Files            1382       1382                
  Lines          116619     116620         +1     
  Branches        20444      20444                
==================================================
- Hits            78144      78143         -1     
- Misses          32039      32040         +1     
- Partials         6436       6437         +1     

Files with missing lines	Coverage Δ
...a/org/codehaus/groovy/runtime/IOGroovyMethods.java	89.7638% <100.0000%> (+0.0269%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eric-milles]

What if writeTo is never called? Is it bad form to add use finalize?

[blackdrag]

Frankly the general Writable-concept has its problems. I was fixing the issue in GROOVY-8373 only. If you look at:

• groovy/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java

  Line 1450 in 2d9c467

  public static void filterLine(Reader reader, Writer writer, @ClosureParams(value=SimpleType.class, options="java.lang.String") Closure closure) throws IOException {

• groovy/src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java

  Line 1490 in 2d9c467

  public static Writable filterLine(Reader reader, @ClosureParams(value=SimpleType.class, options="java.lang.String") final Closure closure) {

then you have one readline that closes the writer, while the other returns a writer, which you are, I guess, supposed to reuse. And in that manner it would be wrong to have it closed in the method I changed. But does the API make sense like this? Well, not part of the issue as I said ;)

[eric-milles]

Certainly this change is a step in the right direction. Is it possible to delay the creation of the buffered reader until writeTo is called? Does the javadoc state that the reader argument is to be closed by the caller?

[eric-milles]

The void filterLine(Reader,Writer,... variant explicitly states that it closes the reader and writer.

groovy/src/main/java/org/codehaus/groovy/runtime/IOGroovyMethods.java

Line 1450 in f3b6fdd

* Both Reader and Writer are closed after the operation.

The Writable filterLine(Reader,...) variant does not say this. Maybe it is enough to add a comment saying reader is not closed. The internal BufferedReader only maintains a reference to the input reader and a char array. The array will be GC'd when no references to the Writable AIC remain. Is that not sufficient?

[blackdrag]

Since we "read all available lines" form the reader I think it is better to close it. Also aligns more with the other method. Yes, the BufferedReader creation can be moved into writeTo. I think that is a good idea.