8220477: Channels.newWriter() does not close if underlying channel throws an IOException

[Brian Burkhalter]

> On Mar 29, 2019, at 7:34 AM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
> 
>> On Mar 29, 2019, at 12:35 AM, Alan Bateman <Alan.Bateman at oracle.com <mailto:Alan.Bateman at oracle.com>> wrote:
>> 
>> On 28/03/2019 21:29, Brian Burkhalter wrote:
>>> For the issue [1] please review the proposed patch [2]. The change is to catch an IOException thrown by implClose() and mark the {en,de}coder as closed anyway. Lots of lines of test for a small change!
>>> 
>> Looks good. As you are there, would you mind fixing formatting in StreamDecoder? I can't tell what happened to it but it looks like the intendation has been removed from L230+ onwards (I only noticed it when looking at the implClose at the end of file).
> 
> Yes I can fix that.
> 
> After I posted this RFR, I ran the patch through all the testing tiers and with this patch the test
> 
> sun/nio/cs/StreamEncoderClose.java
> 
> fails at line 50. I need to think whether this indicates a problem with the patch or the StreamEncoderClose test is incorrect.

Revisiting the failure mentioned above, it turns out that this is because a StreamEncoder for an OutputStream does not flush the stream before closing it. It seems like it should. (I don’t know whether this should perhaps be the subject of a separate issue.) The webrev [1] for the current issue 8220477 is updated to do this. The difference versus the previous webrev is [2]. It might be overkill to handle the IOException from OutputStream.flush() instead of just ignoring it, I’m not sure.

Thanks,

Brian

[1] http://cr.openjdk.java.net/~bpb/8220477/webrev.01
[2] diff vs. webrev.00

--- a/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java
+++ b/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java
@@ -346,8 +346,26 @@
                 writeBytes();
             if (ch != null)
                 ch.close();
-            else
-                out.close();
+            else {
+                IOException flushException = null;
+                try {
+                    out.flush();
+                } catch (IOException fe) {
+                    flushException = fe;
+                } finally {
+                    try {
+                        out.close();
+                    } catch (IOException closeException) {
+                        if (flushException != null) {
+                            closeException.addSuppressed(flushException);
+                        }
+                        throw closeException;
+                    }
+                    if (flushException!= null) {
+                        throw flushException;
+                    }
+                }
+            }
         } catch (IOException x) {
             encoder.reset();
             throw x;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190411/6c126b3b/attachment-0001.html>