On 05/12/14 11:38, Pavel Rappo wrote:
+1, I couldn’t say better
Right. This bug should only try to address the change in behavior that
was inadvertently introduced by 7015589[1][2].
I don't see any reason to make 'closed' protected ( part of the public
Java SE API ), something like:
diff --git a/src/java.base/share/classes/java/io/FilterOutputStream.java
b/src/java.base/share/classes/java/io/FilterOutputStream.java
--- a/src/java.base/share/classes/java/io/FilterOutputStream.java
+++ b/src/java.base/share/classes/java/io/FilterOutputStream.java
@@ -48,6 +48,8 @@
*/
protected OutputStream out;
+ private boolean closed; // false
+
/**
* Creates an output stream filter built on top of the specified
* underlying output stream.
@@ -154,6 +156,9 @@
*/
@SuppressWarnings("try")
public void close() throws IOException {
+ if (closed)
+ return;
+ closed = true;
try (OutputStream ostream = out) {
flush();
}
diff --git a/test/java/io/etc/FailingFlushAndClose.java
b/test/java/io/etc/FailingFlushAndClose.java
--- a/test/java/io/etc/FailingFlushAndClose.java
+++ b/test/java/io/etc/FailingFlushAndClose.java
@@ -25,7 +25,7 @@
/**
* @test
- * @bug 7015589
+ * @bug 7015589 8054565
* @summary Test that buffering streams are considered closed even
when the
* close or flush from the underlying stream fails.
*/
@@ -165,7 +165,7 @@
}
}
- static void testFailingClose(InputStream in) throws IOException {
+ static InputStream testFailingClose(InputStream in) throws
IOException {
System.out.println(in.getClass());
in.read(new byte[100]);
try {
@@ -176,9 +176,10 @@
in.read(new byte[100]);
fail("read did not fail");
} catch (IOException expected) { }
+ return in;
}
- static void testFailingClose(OutputStream out) throws IOException {
+ static OutputStream testFailingClose(OutputStream out) throws
IOException {
System.out.println(out.getClass());
out.write(1);
try {
@@ -190,9 +191,10 @@
if (!(out instanceof BufferedOutputStream))
fail("write did not fail");
} catch (IOException expected) { }
+ return out;
}
- static void testFailingFlush(OutputStream out) throws IOException {
+ static OutputStream testFailingFlush(OutputStream out) throws
IOException {
System.out.println(out.getClass());
out.write(1);
try {
@@ -206,9 +208,27 @@
fail("close did not fail");
} catch (IOException expected) { }
}
+ return out;
}
- static void testFailingClose(Reader r) throws IOException {
+ static void closeAgain(InputStream in) throws IOException {
+ // assert the given stream should already be closed.
+ try {
+ in.close();
+ } catch (IOException expected) {
+ fail("unexpected IOException from subsequent close");
+ }
+ }
+ static void closeAgain(OutputStream out) throws IOException {
+ // assert the given stream should already be closed.
+ try {
+ out.close();
+ } catch (IOException expected) {
+ fail("unexpected IOException from subsequent close");
+ }
+ }
+
+ static Reader testFailingClose(Reader r) throws IOException {
System.out.println(r.getClass());
r.read(new char[100]);
try {
@@ -219,9 +239,10 @@
r.read(new char[100]);
fail("read did not fail");
} catch (IOException expected) { }
+ return r;
}
- static void testFailingClose(Writer w) throws IOException {
+ static Writer testFailingClose(Writer w) throws IOException {
System.out.println(w.getClass());
w.write("message");
try {
@@ -232,9 +253,10 @@
w.write("another message");
fail("write did not fail");
} catch (IOException expected) { }
+ return w;
}
- static void testFailingFlush(Writer w) throws IOException {
+ static Writer testFailingFlush(Writer w) throws IOException {
System.out.println(w.getClass());
w.write("message");
try {
@@ -249,18 +271,38 @@
fail("close did not fail");
} catch (IOException expected) { }
}
+ return w;
+ }
+
+ static Reader closeAgain(Reader r) throws IOException {
+ // assert the given stream should already be closed.
+ try {
+ r.close();
+ } catch (IOException expected) {
+ fail("unexpected IOException from subsequent close");
+ }
+ return r;
+ }
+ static Writer closeAgain(Writer w) throws IOException {
+ // assert the given stream should already be closed.
+ try {
+ w.close();
+ } catch (IOException expected) {
+ fail("unexpected IOException from subsequent close");
+ }
+ return w;
}
public static void main(String[] args) throws IOException {
- testFailingClose(new BufferedInputStream(new
FailingCloseInputStream()));
- testFailingClose(new BufferedOutputStream(new
FailingCloseOutputStream()));
+ closeAgain(testFailingClose(new BufferedInputStream(new
FailingCloseInputStream())));
+ closeAgain(testFailingClose(new BufferedOutputStream(new
FailingCloseOutputStream())));
- testFailingClose(new BufferedReader(new FailingCloseReader()));
- testFailingClose(new BufferedWriter(new FailingCloseWriter()));
+ closeAgain(testFailingClose(new BufferedReader(new
FailingCloseReader())));
+ closeAgain(testFailingClose(new BufferedWriter(new
FailingCloseWriter())));
- testFailingFlush(new BufferedOutputStream(new
FailingFlushOutputStream()));
- testFailingFlush(new BufferedWriter(new FailingFlushWriter()));
+ closeAgain(testFailingFlush(new BufferedOutputStream(new
FailingFlushOutputStream())));
+ closeAgain(testFailingFlush(new BufferedWriter(new
FailingFlushWriter())));
if (failed > 0)
throw new RuntimeException(failed + " test(s) failed - see
log for details");
-Chris.
[1] https://bugs.openjdk.java.net/browse/JDK-7015589
[2] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/759aa847dcaf
-Pavel
On 5 Dec 2014, at 01:18, Vitaly Davidovich <vita...@gmail.com> wrote:
Attempting to make close() atomic just makes the next reader
confused when the rest of the class isn't and may also penalize single
threaded callers of close().
