Skip to content

Commit

Permalink
[netty#2769] Fix regression when writing different message types
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Norman Maurer committed Aug 15, 2014
1 parent a241648 commit 050e7ed
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
Expand Down Expand Up @@ -83,6 +84,7 @@ public void testFileRegionVoidPromiseNotAutoRead(ServerBootstrap sb, Bootstrap c

private static void testFileRegion0(
ServerBootstrap sb, Bootstrap cb, boolean voidPromise, final boolean autoRead) throws Throwable {
final int bufferSize = 1024;
final File file = File.createTempFile("netty-", ".tmp");
file.deleteOnExit();

Expand All @@ -96,7 +98,7 @@ private static void testFileRegion0(
}

// .. 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.
for (int i = random.nextInt(8192); i > 0; i --) {
Expand Down Expand Up @@ -130,10 +132,16 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
Channel sc = sb.bind().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) {
assertEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize), cc.voidPromise()));
assertEquals(cc.voidPromise(), cc.writeAndFlush(region, cc.voidPromise()));
} else {
assertNotEquals(cc.voidPromise(), cc.write(Unpooled.wrappedBuffer(data, 0, bufferSize)));
assertNotEquals(cc.voidPromise(), cc.writeAndFlush(region));
}
while (sh.counter < data.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,18 +297,17 @@ protected void doWrite(ChannelOutboundBuffer in) throws Exception {
}

if (done) {
// Release all buffers
for (int i = size; i > 0; i --) {
// Release all written buffers.
//
// 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();
in.progress(buf.readableBytes());
in.remove();
}

// Finish the write loop if no new messages were flushed by in.remove().
if (in.isEmpty()) {
clearOpWrite();
break;
}
} else {
// Did not write all buffers completely.
// Release the fully written buffers and update the indexes of the partially written buffer.
Expand Down

0 comments on commit 050e7ed

Please sign in to comment.