7015589: (spec) BufferedWriter.close leaves stream open if close of underlying Writer fails
Reviewed-by: forax, mduigou
--- a/jdk/src/share/classes/java/io/BufferedReader.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/src/share/classes/java/io/BufferedReader.java Thu Aug 18 16:47:20 2011 +0100
@@ -514,9 +514,12 @@
synchronized (lock) {
if (in == null)
return;
- in.close();
- in = null;
- cb = null;
+ try {
+ in.close();
+ } finally {
+ in = null;
+ cb = null;
+ }
}
}
}
--- a/jdk/src/share/classes/java/io/BufferedWriter.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/src/share/classes/java/io/BufferedWriter.java Thu Aug 18 16:47:20 2011 +0100
@@ -255,15 +255,15 @@
}
}
+ @SuppressWarnings("try")
public void close() throws IOException {
synchronized (lock) {
if (out == null) {
return;
}
- try {
+ try (Writer w = out) {
flushBuffer();
} finally {
- out.close();
out = null;
cb = null;
}
--- a/jdk/src/share/classes/java/io/Closeable.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/src/share/classes/java/io/Closeable.java Thu Aug 18 16:47:20 2011 +0100
@@ -42,6 +42,12 @@
* with it. If the stream is already closed then invoking this
* method has no effect.
*
+ * <p> As noted in {@link AutoCloseable#close()}, cases where the
+ * close may fail require careful attention. It is strongly advised
+ * to relinquish the underlying resources and to internally
+ * <em>mark</em> the {@code Closeable} as closed, prior to throwing
+ * the {@code IOException}.
+ *
* @throws IOException if an I/O error occurs
*/
public void close() throws IOException;
--- a/jdk/src/share/classes/java/io/FilterOutputStream.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/src/share/classes/java/io/FilterOutputStream.java Thu Aug 18 16:47:20 2011 +0100
@@ -152,11 +152,10 @@
* @see java.io.FilterOutputStream#flush()
* @see java.io.FilterOutputStream#out
*/
+ @SuppressWarnings("try")
public void close() throws IOException {
- try {
- flush();
- } catch (IOException ignored) {
+ try (OutputStream ostream = out) {
+ flush();
}
- out.close();
}
}
--- a/jdk/src/share/classes/java/lang/AutoCloseable.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/src/share/classes/java/lang/AutoCloseable.java Thu Aug 18 16:47:20 2011 +0100
@@ -43,6 +43,15 @@
* throw more specific exceptions, or to throw no exception at all
* if the close operation cannot fail.
*
+ * <p> Cases where the close operation may fail require careful
+ * attention by implementers. It is strongly advised to relinquish
+ * the underlying resources and to internally <em>mark</em> the
+ * resource as closed, prior to throwing the exception. The {@code
+ * close} method is unlikely to be invoked more than once and so
+ * this ensures that the resources are released in a timely manner.
+ * Furthermore it reduces problems that could arise when the resource
+ * wraps, or is wrapped, by another resource.
+ *
* <p><em>Implementers of this interface are also strongly advised
* to not have the {@code close} method throw {@link
* InterruptedException}.</em>
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/io/etc/FailingFlushAndClose.java Thu Aug 18 16:47:20 2011 +0100
@@ -0,0 +1,268 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.*;
+
+/**
+ * @test
+ * @bug 7015589
+ * @summary Test that buffering streams are considered closed even when the
+ * close or flush from the underlying stream fails.
+ */
+
+public class FailingFlushAndClose {
+
+ static int failed;
+
+ static void fail(String msg) {
+ System.err.println("FAIL: " + msg);
+ failed++;
+ }
+
+ static void failWithIOE(String msg) throws IOException {
+ fail(msg);
+ throw new IOException(msg);
+ }
+
+ static class FailingCloseInputStream extends InputStream {
+ boolean closed;
+ @Override
+ public int read()throws IOException {
+ if (closed)
+ failWithIOE("input stream is closed");
+ return 1;
+ }
+ @Override
+ public void close() throws IOException {
+ if (!closed) {
+ closed = true;
+ throw new IOException("close failed");
+ }
+ }
+ }
+
+ static class FailingCloseOutputStream extends OutputStream {
+ boolean closed;
+ @Override
+ public void write(int b) throws IOException {
+ if (closed)
+ failWithIOE("output stream is closed");
+ }
+ @Override
+ public void flush() throws IOException {
+ if (closed)
+ failWithIOE("output stream is closed");
+ }
+ @Override
+ public void close() throws IOException {
+ if (!closed) {
+ closed = true;
+ throw new IOException("close failed");
+ }
+ }
+ }
+
+ static class FailingFlushOutputStream extends OutputStream {
+ boolean closed;
+ @Override
+ public void write(int b) throws IOException {
+ if (closed)
+ failWithIOE("output stream is closed");
+ }
+ @Override
+ public void flush() throws IOException {
+ if (closed) {
+ failWithIOE("output stream is closed");
+ } else {
+ throw new IOException("flush failed");
+ }
+ }
+ @Override
+ public void close() throws IOException {
+ closed = true;
+ }
+ }
+
+ static class FailingCloseReader extends Reader {
+ boolean closed;
+ @Override
+ public int read(char[] cbuf, int off, int len) throws IOException {
+ if (closed)
+ failWithIOE("reader is closed");
+ return 1;
+ }
+ @Override
+ public void close() throws IOException {
+ if (!closed) {
+ closed = true;
+ throw new IOException("close failed");
+ }
+ }
+ }
+
+ static class FailingCloseWriter extends Writer {
+ boolean closed;
+ @Override
+ public void write(char[] cbuf, int off, int len) throws IOException {
+ if (closed)
+ failWithIOE("writer is closed");
+ }
+ @Override
+ public void flush() throws IOException {
+ if (closed)
+ failWithIOE("writer is closed");
+ }
+ @Override
+ public void close() throws IOException {
+ if (!closed) {
+ closed = true;
+ throw new IOException("close failed");
+ }
+ }
+ }
+
+ static class FailingFlushWriter extends Writer {
+ boolean closed;
+ @Override
+ public void write(char[] cbuf, int off, int len) throws IOException {
+ if (closed)
+ failWithIOE("writer is closed");
+ }
+ @Override
+ public void flush() throws IOException {
+ if (closed) {
+ failWithIOE("writer is closed");
+ } else {
+ throw new IOException("flush failed");
+ }
+ }
+ @Override
+ public void close() throws IOException {
+ if (!closed) {
+ closed = true;
+ throw new IOException("close failed");
+ }
+ }
+ }
+
+ static void testFailingClose(InputStream in) throws IOException {
+ System.out.println(in.getClass());
+ in.read(new byte[100]);
+ try {
+ in.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ try {
+ in.read(new byte[100]);
+ fail("read did not fail");
+ } catch (IOException expected) { }
+ }
+
+ static void testFailingClose(OutputStream out) throws IOException {
+ System.out.println(out.getClass());
+ out.write(1);
+ try {
+ out.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ try {
+ out.write(1);
+ if (!(out instanceof BufferedOutputStream))
+ fail("write did not fail");
+ } catch (IOException expected) { }
+ }
+
+ static void testFailingFlush(OutputStream out) throws IOException {
+ System.out.println(out.getClass());
+ out.write(1);
+ try {
+ out.flush();
+ fail("flush did not fail");
+ } catch (IOException expected) { }
+ if (out instanceof BufferedOutputStream) {
+ out.write(1);
+ try {
+ out.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ }
+ }
+
+ static void testFailingClose(Reader r) throws IOException {
+ System.out.println(r.getClass());
+ r.read(new char[100]);
+ try {
+ r.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ try {
+ r.read(new char[100]);
+ fail("read did not fail");
+ } catch (IOException expected) { }
+ }
+
+ static void testFailingClose(Writer w) throws IOException {
+ System.out.println(w.getClass());
+ w.write("message");
+ try {
+ w.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ try {
+ w.write("another message");
+ fail("write did not fail");
+ } catch (IOException expected) { }
+ }
+
+ static void testFailingFlush(Writer w) throws IOException {
+ System.out.println(w.getClass());
+ w.write("message");
+ try {
+ w.flush();
+ fail("flush did not fail");
+ } catch (IOException expected) { }
+ if (w instanceof BufferedWriter) {
+ // assume this message will be buffered
+ w.write("another message");
+ try {
+ w.close();
+ fail("close did not fail");
+ } catch (IOException expected) { }
+ }
+ }
+
+ public static void main(String[] args) throws IOException {
+
+ testFailingClose(new BufferedInputStream(new FailingCloseInputStream()));
+ testFailingClose(new BufferedOutputStream(new FailingCloseOutputStream()));
+
+ testFailingClose(new BufferedReader(new FailingCloseReader()));
+ testFailingClose(new BufferedWriter(new FailingCloseWriter()));
+
+ testFailingFlush(new BufferedOutputStream(new FailingFlushOutputStream()));
+ testFailingFlush(new BufferedWriter(new FailingFlushWriter()));
+
+ if (failed > 0)
+ throw new RuntimeException(failed + " test(s) failed - see log for details");
+ }
+}
--- a/jdk/test/java/lang/ProcessBuilder/Basic.java Wed Aug 17 22:47:12 2011 -0700
+++ b/jdk/test/java/lang/ProcessBuilder/Basic.java Thu Aug 18 16:47:20 2011 +0100
@@ -1803,7 +1803,7 @@
p.getInputStream().close();
p.getErrorStream().close();
- p.getOutputStream().close();
+ try { p.getOutputStream().close(); } catch (IOException flushFailed) { }
InputStream[] streams = { p.getInputStream(), p.getErrorStream() };
for (final InputStream in : streams) {