Skip to content

Commit

Permalink
8306308: (ch) Writer created by Channels::newWriter may lose data
Browse files Browse the repository at this point in the history
Reviewed-by: djelinski, alanb
  • Loading branch information
Brian Burkhalter committed Oct 20, 2023
1 parent 77b2394 commit a1a62d9
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 35 deletions.
4 changes: 3 additions & 1 deletion src/java.base/share/classes/java/nio/channels/Channels.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
46 changes: 12 additions & 34 deletions src/java.base/share/classes/sun/nio/cs/StreamEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -271,27 +263,21 @@ 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;

this.bb = ByteBuffer.allocate(INITIAL_BYTE_BUFFER_CAPACITY);
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 {
Expand All @@ -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();
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
102 changes: 102 additions & 0 deletions test/jdk/java/nio/channels/Channels/NewWriter.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
});
}
}

0 comments on commit a1a62d9

Please sign in to comment.