[#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:
parent
77474e76d1
commit
16e2e1b181
@ -276,23 +276,33 @@ 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() {
|
||||||
final Channel channel = channelFactory().newChannel();
|
final Channel channel = channelFactory().newChannel();
|
||||||
@ -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;
|
||||||
|
@ -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());
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
Loading…
x
Reference in New Issue
Block a user