7015589: (spec) BufferedWriter.close leaves stream open if close of underlying Writer fails

Reviewed-by: forax, mduigou

src/share/classes/java/io/BufferedReader.java

@@ -514,9 +514,12 @@ public class BufferedReader extends Reader {
         synchronized (lock) {
             if (in == null)
                 return;
-            in.close();
-            in = null;
-            cb = null;
+            try {
+                in.close();
+            } finally {
+                in = null;
+                cb = null;
+            }
         }
     }
 }

src/share/classes/java/io/BufferedWriter.java

@@ -255,15 +255,15 @@ public class BufferedWriter extends Writer {
         }
     }
 
+    @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;
             }

src/share/classes/java/io/Closeable.java

@@ -42,6 +42,12 @@ public interface Closeable extends AutoCloseable {
      * 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;

src/share/classes/java/io/FilterOutputStream.java

@@ -152,11 +152,10 @@ class FilterOutputStream extends OutputStream {
      * @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();
     }
 }

src/share/classes/java/lang/AutoCloseable.java

@@ -43,6 +43,15 @@ public interface AutoCloseable {
      * 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>

test/java/io/etc/FailingFlushAndClose.java

+/*
+ * 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");
+    }
+}

test/java/lang/ProcessBuilder/Basic.java

@@ -1803,7 +1803,7 @@ public class Basic {
 
             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) {