Hi Pavel,
> On Jul 25, 2019, at 12:43 PM, Pavel Rappo <pavel.ra...@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
