Refactor ChunkedWriteHandler to remove synchronization which can have bad side effects like deadlocks. See #297 and #301

Conflicts:

	handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java
This commit is contained in:
norman 2012-05-04 10:31:06 +02:00 committed by Trustin Lee
parent f8f703e679
commit 280c65a28e

View File

@ -15,30 +15,23 @@
*/ */
package io.netty.handler.stream; package io.netty.handler.stream;
import static io.netty.channel.Channels.*;
import java.io.IOException;
import java.nio.channels.ClosedChannelException;
import java.util.Queue;
import io.netty.buffer.ChannelBuffers; import io.netty.buffer.ChannelBuffers;
import io.netty.channel.Channel; import io.netty.channel.Channel;
import io.netty.channel.ChannelDownstreamHandler;
import io.netty.channel.ChannelEvent;
import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPipeline; import io.netty.channel.ChannelPipeline;
import io.netty.channel.ChannelStateEvent;
import io.netty.channel.ChannelUpstreamHandler;
import io.netty.channel.Channels;
import io.netty.channel.LifeCycleAwareChannelHandler;
import io.netty.channel.MessageEvent;
import io.netty.logging.InternalLogger; import io.netty.logging.InternalLogger;
import io.netty.logging.InternalLoggerFactory; import io.netty.logging.InternalLoggerFactory;
import io.netty.util.internal.QueueFactory; import io.netty.util.internal.QueueFactory;
import java.io.IOException;
import java.nio.channels.Channels;
import java.nio.channels.ClosedChannelException;
import java.util.Queue;
import java.util.concurrent.atomic.AtomicBoolean;
/** /**
* A {@link ChannelHandler} that adds support for writing a large data stream * A {@link ChannelHandler} that adds support for writing a large data stream
* asynchronously neither spending a lot of memory nor getting * asynchronously neither spending a lot of memory nor getting
@ -79,7 +72,8 @@ public class ChunkedWriteHandler implements ChannelUpstreamHandler, ChannelDowns
private final Queue<MessageEvent> queue = QueueFactory.createQueue(MessageEvent.class); private final Queue<MessageEvent> queue = QueueFactory.createQueue(MessageEvent.class);
private ChannelHandlerContext ctx; private volatile ChannelHandlerContext ctx;
private final AtomicBoolean flush = new AtomicBoolean(false);
private MessageEvent currentEvent; private MessageEvent currentEvent;
/** /**
@ -183,8 +177,14 @@ public class ChunkedWriteHandler implements ChannelUpstreamHandler, ChannelDowns
} }
} }
private synchronized void flush(ChannelHandlerContext ctx, boolean fireNow) throws Exception { private void flush(ChannelHandlerContext ctx, boolean fireNow) throws Exception {
final Channel channel = ctx.channel(); boolean acquired = false;
final Channel channel = ctx.getChannel();
// use CAS to see if the have flush already running, if so we don't need to take futher actions
if (acquired = flush.compareAndSet(false, true)) {
try {
if (!channel.isConnected()) { if (!channel.isConnected()) {
discard(ctx, fireNow); discard(ctx, fireNow);
} }
@ -206,7 +206,7 @@ public class ChunkedWriteHandler implements ChannelUpstreamHandler, ChannelDowns
final MessageEvent currentEvent = this.currentEvent; final MessageEvent currentEvent = this.currentEvent;
Object m = currentEvent.getMessage(); Object m = currentEvent.getMessage();
if (m instanceof ChunkedInput) { if (m instanceof ChunkedInput) {
ChunkedInput chunks = (ChunkedInput) m; final ChunkedInput chunks = (ChunkedInput) m;
Object chunk; Object chunk;
boolean endOfInput; boolean endOfInput;
boolean suspend; boolean suspend;
@ -243,16 +243,26 @@ public class ChunkedWriteHandler implements ChannelUpstreamHandler, ChannelDowns
ChannelFuture writeFuture; ChannelFuture writeFuture;
if (endOfInput) { if (endOfInput) {
this.currentEvent = null; this.currentEvent = null;
closeInput(chunks);
writeFuture = currentEvent.getFuture(); writeFuture = currentEvent.getFuture();
// Register a listener which will close the input once the write is complete. This is needed because the Chunk may have
// some resource bound that can not be closed before its not written
//
// See https://github.com/netty/netty/issues/303
writeFuture.addListener(new ChannelFutureListener() {
@Override
public void operationComplete(ChannelFuture future) throws Exception {
closeInput(chunks);
}
});
} else { } else {
writeFuture = future(channel); writeFuture = future(channel);
writeFuture.addListener(new ChannelFutureListener() { writeFuture.addListener(new ChannelFutureListener() {
@Override @Override
public void operationComplete(ChannelFuture future) public void operationComplete(ChannelFuture future) throws Exception {
throws Exception {
if (!future.isSuccess()) { if (!future.isSuccess()) {
currentEvent.getFuture().setFailure(future.cause()); currentEvent.getFuture().setFailure(future.getCause());
closeInput((ChunkedInput) currentEvent.getMessage()); closeInput((ChunkedInput) currentEvent.getMessage());
} }
} }
@ -274,6 +284,16 @@ public class ChunkedWriteHandler implements ChannelUpstreamHandler, ChannelDowns
break; break;
} }
} }
} finally {
// mark the flush as done
flush.set(false);
}
}
if (acquired && !channel.isConnected() || channel.isWritable() && !queue.isEmpty()) {
flush(ctx, fireNow);
}
} }
static void closeInput(ChunkedInput chunks) { static void closeInput(ChunkedInput chunks) {