8078891: java.io.SequenceInputStream.close is not atomic and not idempotent

[Brian Burkhalter]

Hi Pavel,

> On Jul 25, 2019, at 12:43 PM, Pavel Rappo <pavel.rappo at oracle.com> wrote:
> 
> Thanks a lot for picking up a bug I filled back in 2015. This looks like a good
> cleanup!

Thanks for looking at it.

> I'm a bit concerned though with the added `finally` block in L98. Could that
> lead to a subtle behavioral change for (an unlikely) case where a `read` method
> exhausts the current stream, then tries to close it, fails (`close` throws
> IOException) and then jumps over to the next stream instead of staying on the
> current one?
> 
> Previously, `read` would be tripping over a closed stream forever.

This concern would be fixed by changing the proposed patch as

--- a/src/java.base/share/classes/java/io/SequenceInputStream.java
+++ b/src/java.base/share/classes/java/io/SequenceInputStream.java
@@ -91,13 +91,10 @@
      * Continues reading in the next stream if an EOF is reached.
      */
     final void nextStream() throws IOException {
-        try {
-            if (in != null) {
-                in.close();
-            }
-        } finally {
-            peekNextStream();
+        if (in != null) {
+            in.close();
         }
+        peekNextStream();
     }
 
     private void peekNextStream() {
@@ -233,6 +230,7 @@
                 } else {
                     ioe.addSuppressed(e);
                 }
+                peekNextStream();
             }
         } while (in != null);
         if (ioe != null) {

That is to say reverting the nextStream() change and adding peekNextStream() in the catch branch of close(). I actually had it this way in a version I did not post.

The scenario you suggest however would seem to be somewhat of a coding error by the caller. I would think that once a sequence stream were created the component streams should not be used.

> Theoretically speaking, we might have a case of a "non-sticky" error in the
> InputStream. Try to read, fail, try to read again -- you might get lucky.

Thanks,

Brian