[#2586] Use correct EventExecutor to notify for bind failures on late registration

Motivation:

We used the wrong EventExecutor to notify for bind failures if a late registration was done.

Modifications:

Use the correct EventExecutor to notify and only use the GlobelEventExecutor if the registration fails itself.

Result:

The correct Thread will do the notification.
This commit is contained in:
Norman Maurer 2014-08-20 16:32:39 +02:00
parent 5386c851c8
commit 62e26a2818
2 changed files with 29 additions and 9 deletions

View File

@ -276,22 +276,32 @@ public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C ext
return regFuture; return regFuture;
} }
final ChannelPromise promise;
if (regFuture.isDone()) { if (regFuture.isDone()) {
promise = channel.newPromise(); // At this point we know that the registration was complete and succesful.
ChannelPromise promise = channel.newPromise();
doBind0(regFuture, channel, localAddress, promise); doBind0(regFuture, channel, localAddress, promise);
return promise;
} else { } else {
// Registration future is almost always fulfilled already, but just in case it's not. // Registration future is almost always fulfilled already, but just in case it's not.
promise = new PendingRegistrationPromise(channel); final PendingRegistrationPromise promise = new PendingRegistrationPromise(channel);
regFuture.addListener(new ChannelFutureListener() { regFuture.addListener(new ChannelFutureListener() {
@Override @Override
public void operationComplete(ChannelFuture future) throws Exception { public void operationComplete(ChannelFuture future) throws Exception {
Throwable cause = future.cause();
if (cause != null) {
// Registration on the EventLoop failed so fail the ChannelPromise directly to not cause an
// IllegalStateException once we try to access the EventLoop of the Channel.
promise.setFailure(cause);
} else {
// Registration was successful, so set the correct executor to use.
// See https://github.com/netty/netty/issues/2586
promise.executor = channel.eventLoop();
}
doBind0(regFuture, channel, localAddress, promise); doBind0(regFuture, channel, localAddress, promise);
} }
}); });
return promise;
} }
return promise;
} }
final ChannelFuture initAndRegister() { final ChannelFuture initAndRegister() {
@ -455,18 +465,22 @@ public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C ext
} }
private static final class PendingRegistrationPromise extends DefaultChannelPromise { private static final class PendingRegistrationPromise extends DefaultChannelPromise {
// Is set to the correct EventExecutor once the registration was successful. Otherwise it will
// stay null and so the GlobalEventExecutor.INSTANCE will be used for notifications.
private volatile EventExecutor executor;
private PendingRegistrationPromise(Channel channel) { private PendingRegistrationPromise(Channel channel) {
super(channel); super(channel);
} }
@Override @Override
protected EventExecutor executor() { protected EventExecutor executor() {
if (channel().isRegistered()) { EventExecutor executor = this.executor;
// If the registration was a success we can just call super.executor() which will return if (executor != null) {
// channel.eventLoop(). // If the registration was a success executor is set.
// //
// See https://github.com/netty/netty/issues/2586 // See https://github.com/netty/netty/issues/2586
return super.executor(); return executor;
} }
// The registration failed so we can only use the GlobalEventExecutor as last resort to notify. // The registration failed so we can only use the GlobalEventExecutor as last resort to notify.
return GlobalEventExecutor.INSTANCE; return GlobalEventExecutor.INSTANCE;

View File

@ -178,11 +178,17 @@ public class BootstrapTest {
return new LocalServerChannel() { return new LocalServerChannel() {
@Override @Override
public ChannelFuture bind(SocketAddress localAddress) { public ChannelFuture bind(SocketAddress localAddress) {
// Close the Channel to emulate what NIO and others impl do on bind failure
// See https://github.com/netty/netty/issues/2586
close();
return newFailedFuture(new SocketException()); return newFailedFuture(new SocketException());
} }
@Override @Override
public ChannelFuture bind(SocketAddress localAddress, ChannelPromise promise) { public ChannelFuture bind(SocketAddress localAddress, ChannelPromise promise) {
// Close the Channel to emulate what NIO and others impl do on bind failure
// See https://github.com/netty/netty/issues/2586
close();
return promise.setFailure(new SocketException()); return promise.setFailure(new SocketException());
} }
}; };