[#2769] Fix regression when writing different message types

Motivation:

Due a regression NioSocketChannel.doWrite(...) will throw a ClassCastException if you do something like:

channel.write(bytebuf);
channel.write(fileregion);
channel.flush();

Modifications:

Correctly handle writing of different message types by using the correct message count while loop over them.

Result:

No more ClassCastException
This commit is contained in:
Norman Maurer 2014-08-15 11:53:17 +02:00
parent b3c1904cc9
commit dcfdad9e9e
2 changed files with 17 additions and 10 deletions

View File

@ -18,6 +18,7 @@ package io.netty.testsuite.transport.socket;
import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap; import io.netty.bootstrap.ServerBootstrap;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel; import io.netty.channel.Channel;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandler; import io.netty.channel.ChannelInboundHandler;
@ -83,6 +84,7 @@ public class SocketFileRegionTest extends AbstractSocketTest {
private static void testFileRegion0( private static void testFileRegion0(
ServerBootstrap sb, Bootstrap cb, boolean voidPromise, final boolean autoRead) throws Throwable { ServerBootstrap sb, Bootstrap cb, boolean voidPromise, final boolean autoRead) throws Throwable {
final int bufferSize = 1024;
final File file = File.createTempFile("netty-", ".tmp"); final File file = File.createTempFile("netty-", ".tmp");
file.deleteOnExit(); file.deleteOnExit();
@ -96,7 +98,7 @@ public class SocketFileRegionTest extends AbstractSocketTest {
} }
// .. and here comes the real data to transfer. // .. and here comes the real data to transfer.
out.write(data); out.write(data, bufferSize, data.length - bufferSize);
// .. and then some extra data which is not supposed to be transferred. // .. and then some extra data which is not supposed to be transferred.
for (int i = random.nextInt(8192); i > 0; i --) { for (int i = random.nextInt(8192); i > 0; i --) {
@ -130,10 +132,16 @@ public class SocketFileRegionTest extends AbstractSocketTest {
Channel sc = sb.bind().sync().channel(); Channel sc = sb.bind().sync().channel();
Channel cc = cb.connect().sync().channel(); Channel cc = cb.connect().sync().channel();
FileRegion region = new DefaultFileRegion(new FileInputStream(file).getChannel(), startOffset, data.length); FileRegion region = new DefaultFileRegion(
new FileInputStream(file).getChannel(), startOffset, data.length - bufferSize);
// Do write ByteBuf and FileRegion to be sure that mixed writes work
//
// See https://github.com/netty/netty/issues/2769
if (voidPromise) { if (voidPromise) {
assertEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize), cc.voidPromise()));
assertEquals(cc.voidPromise(), cc.writeAndFlush(region, cc.voidPromise())); assertEquals(cc.voidPromise(), cc.writeAndFlush(region, cc.voidPromise()));
} else { } else {
assertNotEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize)));
assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region)); assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region));
} }
while (sh.counter < data.length) { while (sh.counter < data.length) {

View File

@ -298,18 +298,17 @@ public class NioSocketChannel extends AbstractNioByteChannel implements io.netty
} }
if (done) { if (done) {
// Release all buffers // Release all written buffers.
for (int i = size; i > 0; i --) { //
// It's important to loop only over nioBufferCnt as there may be other messages in the
// ChannelOutboundBuffer that are not of type ByteBuf and so was not included for gathering-write.
//
// See https://github.com/netty/netty/issues/2769
for (int i = nioBufferCnt; i > 0; i --) {
final ByteBuf buf = (ByteBuf) in.current(); final ByteBuf buf = (ByteBuf) in.current();
in.progress(buf.readableBytes()); in.progress(buf.readableBytes());
in.remove(); in.remove();
} }
// Finish the write loop if no new messages were flushed by in.remove().
if (in.isEmpty()) {
clearOpWrite();
break;
}
} else { } else {
// Did not write all buffers completely. // Did not write all buffers completely.
// Release the fully written buffers and update the indexes of the partially written buffer. // Release the fully written buffers and update the indexes of the partially written buffer.