From a1a62d9964393533eeab269d1f405c8f0db1e900 Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Fri, 20 Oct 2023 21:12:28 +0000 Subject: [PATCH] 8306308: (ch) Writer created by Channels::newWriter may lose data Reviewed-by: djelinski, alanb --- .../classes/java/nio/channels/Channels.java | 4 +- .../classes/sun/nio/cs/StreamEncoder.java | 46 +++----- .../java/nio/channels/Channels/NewWriter.java | 102 ++++++++++++++++++ 3 files changed, 117 insertions(+), 35 deletions(-) create mode 100644 test/jdk/java/nio/channels/Channels/NewWriter.java diff --git a/src/java.base/share/classes/java/nio/channels/Channels.java b/src/java.base/share/classes/java/nio/channels/Channels.java index 80314d7447f..af3c6bdff73 100644 --- a/src/java.base/share/classes/java/nio/channels/Channels.java +++ b/src/java.base/share/classes/java/nio/channels/Channels.java @@ -541,7 +541,9 @@ public static Writer newWriter(WritableByteChannel ch, int minBufferCap) { Objects.requireNonNull(ch, "ch"); - return StreamEncoder.forEncoder(ch, enc.reset(), minBufferCap); + Objects.requireNonNull(enc, "enc"); + OutputStream out = newOutputStream(ch); + return StreamEncoder.forOutputStreamWriter(out, enc.reset()); } /** diff --git a/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java b/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java index f922a787466..3a82030121a 100644 --- a/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java +++ b/src/java.base/share/classes/sun/nio/cs/StreamEncoder.java @@ -31,7 +31,6 @@ import java.io.Writer; import java.nio.ByteBuffer; import java.nio.CharBuffer; -import java.nio.channels.WritableByteChannel; import java.nio.charset.Charset; import java.nio.charset.CharsetEncoder; import java.nio.charset.CoderResult; @@ -79,17 +78,12 @@ public static StreamEncoder forOutputStreamWriter(OutputStream out, return new StreamEncoder(out, lock, enc); } - - // Factory for java.nio.channels.Channels.newWriter - - public static StreamEncoder forEncoder(WritableByteChannel ch, - CharsetEncoder enc, - int minBufferCap) + public static StreamEncoder forOutputStreamWriter(OutputStream out, + CharsetEncoder enc) { - return new StreamEncoder(ch, enc, minBufferCap); + return new StreamEncoder(out, enc); } - // -- Public methods corresponding to those in OutputStreamWriter -- // All synchronization and state/argument checking is done in these public @@ -252,9 +246,7 @@ private boolean isOpen() { private ByteBuffer bb; private final int maxBufferCapacity; - // Exactly one of these is non-null private final OutputStream out; - private final WritableByteChannel ch; // Leftover first char in a surrogate pair private boolean haveLeftoverChar = false; @@ -271,7 +263,6 @@ private StreamEncoder(OutputStream out, Object lock, Charset cs) { private StreamEncoder(OutputStream out, Object lock, CharsetEncoder enc) { super(lock); this.out = out; - this.ch = null; this.cs = enc.charset(); this.encoder = enc; @@ -279,19 +270,14 @@ private StreamEncoder(OutputStream out, Object lock, CharsetEncoder enc) { this.maxBufferCapacity = MAX_BYTE_BUFFER_CAPACITY; } - private StreamEncoder(WritableByteChannel ch, CharsetEncoder enc, int mbc) { - this.out = null; - this.ch = ch; + private StreamEncoder(OutputStream out, CharsetEncoder enc) { + super(); + this.out = out; this.cs = enc.charset(); this.encoder = enc; - if (mbc > 0) { - this.bb = ByteBuffer.allocate(mbc); - this.maxBufferCapacity = mbc; - } else { - this.bb = ByteBuffer.allocate(INITIAL_BYTE_BUFFER_CAPACITY); - this.maxBufferCapacity = MAX_BYTE_BUFFER_CAPACITY; - } + this.bb = ByteBuffer.allocate(INITIAL_BYTE_BUFFER_CAPACITY); + this.maxBufferCapacity = MAX_BYTE_BUFFER_CAPACITY; } private void writeBytes() throws IOException { @@ -302,12 +288,7 @@ private void writeBytes() throws IOException { int rem = (pos <= lim ? lim - pos : 0); if (rem > 0) { - if (ch != null) { - int wc = ch.write(bb); - assert wc == rem : rem; - } else { - out.write(bb.array(), bb.arrayOffset() + pos, rem); - } + out.write(bb.array(), bb.arrayOffset() + pos, rem); } bb.clear(); } @@ -408,13 +389,11 @@ void implFlushBuffer() throws IOException { void implFlush() throws IOException { implFlushBuffer(); - if (out != null) { - out.flush(); - } + out.flush(); } void implClose() throws IOException { - try (ch; out) { + try (out) { flushLeftoverChar(null, true); for (;;) { CoderResult cr = encoder.flush(bb); @@ -430,8 +409,7 @@ void implClose() throws IOException { if (bb.position() > 0) writeBytes(); - if (out != null) - out.flush(); + out.flush(); } catch (IOException x) { encoder.reset(); throw x; diff --git a/test/jdk/java/nio/channels/Channels/NewWriter.java b/test/jdk/java/nio/channels/Channels/NewWriter.java new file mode 100644 index 00000000000..95445f0b84a --- /dev/null +++ b/test/jdk/java/nio/channels/Channels/NewWriter.java @@ -0,0 +1,102 @@ +/* + * Copyright (c) 2023, 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.IOException; +import java.io.Writer; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.StandardSocketOptions; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.IllegalBlockingModeException; +import java.nio.channels.SocketChannel; +import java.nio.channels.WritableByteChannel; +import java.nio.charset.StandardCharsets; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * @test + * @bug 8295797 + * @summary Test behavior of Channels.newWriter for WritableByteChannels + * @run junit NewWriter + */ +public class NewWriter { + private static final String STRING = "test"; + private static final int COUNT = 5; + private static final int EXPECTED = COUNT*STRING.length(); + private int actual = 0; + + @Test + public void oneByteChannel() throws IOException { + try (Writer writer = Channels.newWriter(new WritableByteChannel() { + @Override + public int write(ByteBuffer src) { + System.out.print((char) src.get()); + actual++; + return 1; + } + + @Override + public boolean isOpen() { + return true; + } + + @Override + public void close() { + } + }, StandardCharsets.UTF_8)) { + for (int i = 1; i <= COUNT; i++) { + writer.write(STRING); + writer.flush(); + System.out.println(i); + } + } + assertEquals(EXPECTED, actual); + } + + @Test + public void socketChannel() throws IOException { + Throwable thrown = assertThrows(IllegalBlockingModeException.class, + () -> { + try (ServerSocket ss = new ServerSocket(); + SocketChannel sc = SocketChannel.open()) { + + InetAddress lb = InetAddress.getLoopbackAddress(); + ss.bind(new InetSocketAddress(lb, 0)); + sc.connect(ss.getLocalSocketAddress()); + sc.configureBlocking(false); + sc.setOption(StandardSocketOptions.SO_SNDBUF, 8192); + try (Writer writer = Channels.newWriter(sc, + StandardCharsets.UTF_8)) { + for (int i = 1; i < Integer.MAX_VALUE; i++) { + writer.write("test" + i); + } + } + } + }); + } +}