From 7328cfe58f1f08b89eadda84ac6ce59d6501f1c9 Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 21 Oct 2011 17:56:43 +0200 Subject: [PATCH 1/4] Commit javadocs update which was previous committed to the 3.2 branch --- src/main/java/org/jboss/netty/channel/ChannelEvent.java | 2 ++ .../jboss/netty/channel/SimpleChannelUpstreamHandler.java | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jboss/netty/channel/ChannelEvent.java b/src/main/java/org/jboss/netty/channel/ChannelEvent.java index ca566d5022..2ab0b0b851 100644 --- a/src/main/java/org/jboss/netty/channel/ChannelEvent.java +++ b/src/main/java/org/jboss/netty/channel/ChannelEvent.java @@ -71,6 +71,7 @@ import org.jboss.netty.channel.socket.ServerSocketChannel; * {@code "channelOpen"} * {@link ChannelStateEvent}
(state = {@link ChannelState#OPEN OPEN}, value = {@code true}) * a {@link Channel} is open, but not bound nor connected + * Be aware that this event is fired from within the Boss-Thread so you should not execute any heavy operation in there as it will block the dispatching to other workers! * * * {@code "channelClosed"} @@ -81,6 +82,7 @@ import org.jboss.netty.channel.socket.ServerSocketChannel; * {@code "channelBound"} * {@link ChannelStateEvent}
(state = {@link ChannelState#BOUND BOUND}, value = {@link SocketAddress}) * a {@link Channel} is open and bound to a local address, but not connected + * Be aware that this event is fired from within the Boss-Thread so you should not execute any heavy operation in there as it will block the dispatching to other workers! * * * {@code "channelUnbound"} diff --git a/src/main/java/org/jboss/netty/channel/SimpleChannelUpstreamHandler.java b/src/main/java/org/jboss/netty/channel/SimpleChannelUpstreamHandler.java index 189269c46f..c942b8e7d7 100644 --- a/src/main/java/org/jboss/netty/channel/SimpleChannelUpstreamHandler.java +++ b/src/main/java/org/jboss/netty/channel/SimpleChannelUpstreamHandler.java @@ -151,7 +151,9 @@ public class SimpleChannelUpstreamHandler implements ChannelUpstreamHandler { /** * Invoked when a {@link Channel} is open, but not bound nor connected. - */ + *
+ * Be aware that this event is fired from within the Boss-Thread so you should not execute any heavy operation in there as it will block the dispatching to other workers! + */ public void channelOpen( ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception { ctx.sendUpstream(e); @@ -160,6 +162,8 @@ public class SimpleChannelUpstreamHandler implements ChannelUpstreamHandler { /** * Invoked when a {@link Channel} is open and bound to a local address, * but not connected. + *
+ * Be aware that this event is fired from within the Boss-Thread so you should not execute any heavy operation in there as it will block the dispatching to other workers! */ public void channelBound( ChannelHandlerContext ctx, ChannelStateEvent e) throws Exception { From 783e7562c7621e673a11f4bc39fea0a3e0482919 Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 21 Oct 2011 18:04:18 +0200 Subject: [PATCH 2/4] Fire ChannelConnected events in a Worker Thread. See NETTY-439 --- .../netty/channel/socket/nio/NioAcceptedSocketChannel.java | 1 - .../java/org/jboss/netty/channel/socket/nio/NioWorker.java | 5 +++++ .../netty/channel/socket/oio/OioAcceptedSocketChannel.java | 1 - .../java/org/jboss/netty/channel/socket/oio/OioWorker.java | 5 +++++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioAcceptedSocketChannel.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioAcceptedSocketChannel.java index ad7571ba8a..dff5309d8e 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioAcceptedSocketChannel.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioAcceptedSocketChannel.java @@ -48,6 +48,5 @@ final class NioAcceptedSocketChannel extends NioSocketChannel { setConnected(); fireChannelOpen(this); fireChannelBound(this, getLocalAddress()); - fireChannelConnected(this, getRemoteAddress()); } } diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java b/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java index 0bf1dc51c9..2fedb57256 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/NioWorker.java @@ -782,6 +782,11 @@ class NioWorker implements Runnable { } fireChannelConnected(channel, remoteAddress); } + + // Handle the channelConnected in the worker thread + if (channel instanceof NioAcceptedSocketChannel) { + fireChannelConnected(channel, channel.getRemoteAddress()); + } } } } diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioAcceptedSocketChannel.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioAcceptedSocketChannel.java index d42cb6f0e7..16781e5c1c 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioAcceptedSocketChannel.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioAcceptedSocketChannel.java @@ -63,7 +63,6 @@ class OioAcceptedSocketChannel extends OioSocketChannel { fireChannelOpen(this); fireChannelBound(this, getLocalAddress()); - fireChannelConnected(this, getRemoteAddress()); } @Override diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java index 945927f178..4d5f6f7e9b 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java @@ -50,8 +50,13 @@ class OioWorker implements Runnable { public void run() { channel.workerThread = Thread.currentThread(); final PushbackInputStream in = channel.getInputStream(); + boolean fireOpen = channel instanceof OioAcceptedSocketChannel; while (channel.isOpen()) { + if (fireOpen) { + fireOpen = false; + fireChannelConnected(channel, channel.getRemoteAddress()); + } synchronized (channel.interestOpsLock) { while (!channel.isReadable()) { try { From c6e0162887e7652f7ff4bffebfa288d7d78333b0 Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 21 Oct 2011 18:06:00 +0200 Subject: [PATCH 3/4] Make sure FileRegion.releaseExternalResources() is called after the write was done. See NETTY-440 --- .../jboss/netty/channel/socket/nio/SocketSendBufferPool.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jboss/netty/channel/socket/nio/SocketSendBufferPool.java b/src/main/java/org/jboss/netty/channel/socket/nio/SocketSendBufferPool.java index ba1ab5de83..4c29ee79b3 100644 --- a/src/main/java/org/jboss/netty/channel/socket/nio/SocketSendBufferPool.java +++ b/src/main/java/org/jboss/netty/channel/socket/nio/SocketSendBufferPool.java @@ -306,7 +306,8 @@ final class SocketSendBufferPool { @Override public void release() { - // Unpooled. + // Make sure the FileRegion resource are released otherwise it may cause a FD leak or something similar + file.releaseExternalResources(); } } From 6dd77331ed3364aedcb6da627e6919f2ede232a7 Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 21 Oct 2011 18:11:06 +0200 Subject: [PATCH 4/4] Add support for FileRegion in OIOWorker. See NETTY-441 --- .../netty/channel/socket/oio/OioWorker.java | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java b/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java index 4d5f6f7e9b..82fac7509d 100644 --- a/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java +++ b/src/main/java/org/jboss/netty/channel/socket/oio/OioWorker.java @@ -20,12 +20,15 @@ import static org.jboss.netty.channel.Channels.*; import java.io.OutputStream; import java.io.PushbackInputStream; import java.net.SocketException; +import java.nio.channels.Channels; import java.nio.channels.ClosedChannelException; +import java.nio.channels.WritableByteChannel; import java.util.regex.Pattern; import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelFuture; +import org.jboss.netty.channel.FileRegion; /** * @@ -119,13 +122,39 @@ class OioWorker implements Runnable { } try { - ChannelBuffer a = (ChannelBuffer) message; - int length = a.readableBytes(); - synchronized (out) { - a.getBytes(a.readerIndex(), out, length); + int length = 0; + + // Add support to write a FileRegion. This in fact will not give any performance gain but at least it not fail and + // we did the best to emulate it + if (message instanceof FileRegion) { + FileRegion fr = (FileRegion) message; + try { + synchronized (out) { + WritableByteChannel bchannel = Channels.newChannel(out); + + long i = 0; + while ((i = fr.transferTo(bchannel, length)) > 0) { + length += i; + if (length >= fr.getCount()) { + break; + } + } + } + } finally { + fr.releaseExternalResources(); + + } + } else { + ChannelBuffer a = (ChannelBuffer) message; + length = a.readableBytes(); + synchronized (out) { + a.getBytes(a.readerIndex(), out, length); + } } + fireWriteComplete(channel, length); future.setSuccess(); + } catch (Throwable t) { // Convert 'SocketException: Socket closed' to // ClosedChannelException.