Fix a potential NPE due to the race between a connection attempt and its cancellation

- should fix #2086
This commit is contained in:
Trustin Lee 2014-01-09 19:24:44 +09:00
parent 2338bc52cb
commit 2af1419056

View File

@ -213,6 +213,11 @@ public abstract class AbstractNioChannel extends AbstractChannel {
} }
private void fulfillConnectPromise(ChannelPromise promise, boolean wasActive) { private void fulfillConnectPromise(ChannelPromise promise, boolean wasActive) {
if (promise == null) {
// Closed via cancellation and the promise has been notified already.
return;
}
// trySuccess() will return false if a user cancelled the connection attempt. // trySuccess() will return false if a user cancelled the connection attempt.
boolean promiseSet = promise.trySuccess(); boolean promiseSet = promise.trySuccess();
@ -228,23 +233,27 @@ public abstract class AbstractNioChannel extends AbstractChannel {
} }
} }
private void fulfillConnectPromise(ChannelPromise promise, Throwable cause) {
if (promise == null) {
// Closed via cancellation and the promise has been notified already.
}
// Use tryFailure() instead of setFailure() to avoid the race against cancel().
promise.tryFailure(cause);
closeIfClosed();
}
@Override @Override
public void finishConnect() { public void finishConnect() {
// Note this method is invoked by the event loop only if the connection attempt was // Note this method is invoked by the event loop only if the connection attempt was
// neither cancelled nor timed out. // neither cancelled nor timed out.
assert eventLoop().inEventLoop(); assert eventLoop().inEventLoop();
assert connectPromise != null;
// Cache connect promise as connectPromise will be set to null once it is notified.
// This is needed to prevent a possible NPE
// See https://github.com/netty/netty/issues/2086
ChannelPromise promise = connectPromise;
try { try {
boolean wasActive = isActive(); boolean wasActive = isActive();
doFinishConnect(); doFinishConnect();
fulfillConnectPromise(promise, wasActive); fulfillConnectPromise(connectPromise, wasActive);
} catch (Throwable t) { } catch (Throwable t) {
if (t instanceof ConnectException) { if (t instanceof ConnectException) {
Throwable newT = new ConnectException(t.getMessage() + ": " + requestedRemoteAddress); Throwable newT = new ConnectException(t.getMessage() + ": " + requestedRemoteAddress);
@ -252,9 +261,7 @@ public abstract class AbstractNioChannel extends AbstractChannel {
t = newT; t = newT;
} }
// Use tryFailure() instead of setFailure() to avoid the race against cancel(). fulfillConnectPromise(connectPromise, t);
promise.tryFailure(t);
closeIfClosed();
} finally { } finally {
// Check for null as the connectTimeoutFuture is only created if a connectTimeoutMillis > 0 is used // Check for null as the connectTimeoutFuture is only created if a connectTimeoutMillis > 0 is used
// See https://github.com/netty/netty/issues/1770 // See https://github.com/netty/netty/issues/1770