[PATCH] JDK-8054565: FilterOutputStream.close may throw IOException if called twice and underlying flush or close fails

[Bernd Eckenfels]

Hello,

also using a stream in a multi threaded way is quite unusual most of
the implementations I have seen use a atomic solution. This makes
sense considering the fact that especially the close() might be called
by a timeout/cleanup/finalizer/timer/shutdown thread. Using a
AtomicUpdater would reduce the allocation:


volatile boolean closeFlag;
AtomicBoleanFieldUpdater CLOSEUPDATER = new
AtomicBooleanFieldUpdater<>(FilterOutputStream.class, "closeFlag");


close() {
  if (CLOSEUPDATER.compareAndSet(this, false, true) {
    // this is the first closer
  }
}

Let me add a comment that those stream classes are all heavily
overloaded in all parts of code. I think this kind of change is pretty
risky (and most people fixed this and other close insanities in the
derived methods anyway). Unfortunatelly. (remeber the
SupressionException problem?)

Gruss
Bernd



Am Fri, 5 Dec 2014
09:32:12 +1100 schrieb Nathan Clement <nathan.a.clement at hotmail.com>:

> Hi,
> 
> My apologies - the page http://openjdk.java.net/contribute/ gave me
> the impression that I could send the diff as an attachment.  I've
> included it inline below.
> 
> Regards,
> 
> Nathan
> 
> diff --git a/src/share/classes/java/io/FilterOutputStream.java
> b/src/share/classes/java/io/FilterOutputStream.java ---
> a/src/share/classes/java/io/FilterOutputStream.java +++
> b/src/share/classes/java/io/FilterOutputStream.java @@ -47,6 +47,11 @@
>       * The underlying output stream to be filtered.
>       */
>      protected OutputStream out;
> +    
> +    /**
> +     * Whether this stream has already been closed or not
> +     */
> +    protected boolean closed = false;
>  
>      /**
>       * Creates an output stream filter built on top of the specified
> @@ -154,8 +159,11 @@
>       */
>      @SuppressWarnings("try")
>      public void close() throws IOException {
> -        try (OutputStream ostream = out) {
> -            flush();
> +        if (!closed) {
> +            closed = true;
> +            try (OutputStream ostream = out) {
> +                flush();
> +            }
>          }
>      }
>  }
> diff --git a/test/java/io/FilterOutputStream/CloseTwiceTest.java
> b/test/java/io/FilterOutputStream/CloseTwiceTest.java new file mode
> 100644 --- /dev/null
> +++ b/test/java/io/FilterOutputStream/CloseTwiceTest.java
> @@ -0,0 +1,88 @@
> +/*
> + * 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.
> + */
> +
> +package java.io.FilterOutputStream;
> +
> +import java.io.BufferedOutputStream;
> +import java.io.ByteArrayInputStream;
> +import java.io.ByteArrayOutputStream;
> +import java.io.IOException;
> +import java.io.InputStream;
> +import java.io.OutputStream;
> +import java.lang.String;
> +import java.util.zip.Deflater;
> +import java.util.zip.DeflaterOutputStream;
> +
> +/**
> + * @test
> + * @bug 8054565
> + * @summary Test that closing a FilterOutputStream twice does not
> cause 
> + *    an exception when the underlying stream throws an exception
> when
> + *    close is called before flush.
> + */
> +
> +public class CloseTwiceTest
> +{
> +    /**
> +     * This stream simulates a OracleBlobOutputStream which will
> throw
> +     * an exception if flush is called after close.
> +     */
> +    static class MockBlobOutputStream extends ByteArrayOutputStream
> +    {
> +        private boolean closed;
> +
> +        @Override
> +        public void flush() throws IOException
> +        {
> +            if (closed) {
> +                throw new IOException("Stream closed");
> +            }
> +        }
> +
> +        @Override
> +        public void close() throws IOException
> +        {
> +            closed = true;
> +        }
> +    }
> +
> +    public static void main( final String[] args ) throws Exception
> +    {
> +        try( InputStream bis = new
> ByteArrayInputStream( "Hello".getBytes() );
> +             OutputStream outStream = new MockBlobOutputStream();
> +             BufferedOutputStream bos = new
> BufferedOutputStream( outStream );
> +             DeflaterOutputStream deflaterStream = new
> DeflaterOutputStream(
> +                 bos, new Deflater( 3 ) ) )
> +        {
> +            int len = 0;
> +            final byte[] buf = new byte[1024];
> +            while ((len = bis.read(buf)) >= 0)
> +            {
> +                if ( len > 0 )
> +                {
> +                    deflaterStream.write(buf,0,len);
> +                }
> +            }
> +        }
> +    }
> +}
> 
> 
> > Date: Thu, 4 Dec 2014 09:40:24 -0500
> > From: roger.riggs at oracle.com
> > To: core-libs-dev at openjdk.java.net
> > Subject: Re: [PATCH] JDK-8054565: FilterOutputStream.close may
> > throw	IOException if called twice and underlying flush or
> > close fails
> > 
> > Hi Nathan,
> > 
> > This list removes attachments.
> > Can the diff's be inlined in the email?
> > 
> > Thanks, Roger
> > 
> > On 12/4/2014 1:18 AM, Nathan Clement wrote:
> > > Hi,
> > >
> > > Here is my suggested fix for the FilterOuputStream issue in
> > > JDK-8054565.  I have been running this fix in my applications for
> > > several weeks (by adding it to the bootstrap classpath) and it
> > > solves my issue.  The fix creates a new member field to remember
> > > whether close has already been called on the stream, and if so,
> > > close() does nothing.
> > >
> > > Attached is the output from hg diff -g
> > >
> > > This is my first attempt at a patch for OpenJDK, so please let me
> > > know if I need to change anything.
> > >
> > > Thanks,
> > >
> > > Nathan
> > >   		 	   		
> > 
>