[#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 56fd33df23
commit 246b65c6b6
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;
}
final ChannelPromise promise;
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);
return promise;
} else {
// 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() {
@Override
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);
}
});
return promise;
}
return promise;
}
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 {
// 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) {
super(channel);
}
@Override
protected EventExecutor executor() {
if (channel().isRegistered()) {
// If the registration was a success we can just call super.executor() which will return
// channel.eventLoop().
EventExecutor executor = this.executor;
if (executor != null) {
// If the registration was a success executor is set.
//
// 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.
return GlobalEventExecutor.INSTANCE;

View File

@ -178,11 +178,17 @@ public class BootstrapTest {
return new LocalServerChannel() {
@Override
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());
}
@Override
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());
}
};