Fail fast when DefaultChannelPromise is constructed with null as Channel.

Motivation:

We should fail fast when DefaultChannelPromise is constructed with null as Channel as otherwise it will fail with a NPE once we call setSuccess / setFailure.

Modifications:

Add null check and test.

Result:

Fail fast.
This commit is contained in:
Norman Maurer 2017-12-22 13:19:34 +01:00
parent e6c9ac968d
commit 1740f366eb
5 changed files with 153 additions and 4 deletions

View File

@ -323,9 +323,11 @@ public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C ext
if (channel != null) { if (channel != null) {
// channel can be null if newChannel crashed (eg SocketException("too many open files")) // channel can be null if newChannel crashed (eg SocketException("too many open files"))
channel.unsafe().closeForcibly(); channel.unsafe().closeForcibly();
// as the Channel is not registered yet we need to force the usage of the GlobalEventExecutor
return new DefaultChannelPromise(channel, GlobalEventExecutor.INSTANCE).setFailure(t);
} }
// as the Channel is not registered yet we need to force the usage of the GlobalEventExecutor // as the Channel is not registered yet we need to force the usage of the GlobalEventExecutor
return new DefaultChannelPromise(channel, GlobalEventExecutor.INSTANCE).setFailure(t); return new DefaultChannelPromise(new FailedChannel(), GlobalEventExecutor.INSTANCE).setFailure(t);
} }
ChannelFuture regFuture = config().group().register(channel); ChannelFuture regFuture = config().group().register(channel);

View File

@ -0,0 +1,107 @@
/*
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.bootstrap;
import io.netty.channel.AbstractChannel;
import io.netty.channel.ChannelConfig;
import io.netty.channel.ChannelMetadata;
import io.netty.channel.ChannelOutboundBuffer;
import io.netty.channel.ChannelPromise;
import io.netty.channel.DefaultChannelConfig;
import io.netty.channel.EventLoop;
import java.net.SocketAddress;
final class FailedChannel extends AbstractChannel {
private static final ChannelMetadata METADATA = new ChannelMetadata(false);
private final ChannelConfig config = new DefaultChannelConfig(this);
FailedChannel() {
super(null);
}
@Override
protected AbstractUnsafe newUnsafe() {
return new FailedChannelUnsafe();
}
@Override
protected boolean isCompatible(EventLoop loop) {
return false;
}
@Override
protected SocketAddress localAddress0() {
return null;
}
@Override
protected SocketAddress remoteAddress0() {
return null;
}
@Override
protected void doBind(SocketAddress localAddress) {
throw new UnsupportedOperationException();
}
@Override
protected void doDisconnect() {
throw new UnsupportedOperationException();
}
@Override
protected void doClose() {
throw new UnsupportedOperationException();
}
@Override
protected void doBeginRead() {
throw new UnsupportedOperationException();
}
@Override
protected void doWrite(ChannelOutboundBuffer in) {
throw new UnsupportedOperationException();
}
@Override
public ChannelConfig config() {
return config;
}
@Override
public boolean isOpen() {
return false;
}
@Override
public boolean isActive() {
return false;
}
@Override
public ChannelMetadata metadata() {
return METADATA;
}
private final class FailedChannelUnsafe extends AbstractUnsafe {
@Override
public void connect(SocketAddress remoteAddress, SocketAddress localAddress, ChannelPromise promise) {
promise.setFailure(new UnsupportedOperationException());
}
}
}

View File

@ -21,6 +21,8 @@ import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.Future; import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.GenericFutureListener; import io.netty.util.concurrent.GenericFutureListener;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
/** /**
* The default {@link ChannelPromise} implementation. It is recommended to use {@link Channel#newPromise()} to create * The default {@link ChannelPromise} implementation. It is recommended to use {@link Channel#newPromise()} to create
* a new {@link ChannelPromise} rather than calling the constructor explicitly. * a new {@link ChannelPromise} rather than calling the constructor explicitly.
@ -37,7 +39,7 @@ public class DefaultChannelPromise extends DefaultPromise<Void> implements Chann
* the {@link Channel} associated with this future * the {@link Channel} associated with this future
*/ */
public DefaultChannelPromise(Channel channel) { public DefaultChannelPromise(Channel channel) {
this.channel = channel; this.channel = checkNotNull(channel, "channel");
} }
/** /**
@ -48,7 +50,7 @@ public class DefaultChannelPromise extends DefaultPromise<Void> implements Chann
*/ */
public DefaultChannelPromise(Channel channel, EventExecutor executor) { public DefaultChannelPromise(Channel channel, EventExecutor executor) {
super(executor); super(executor);
this.channel = channel; this.channel = checkNotNull(channel, "channel");
} }
@Override @Override

View File

@ -290,7 +290,7 @@ public class BootstrapTest {
// Should fail with the RuntimeException. // Should fail with the RuntimeException.
assertThat(connectFuture.await(10000), is(true)); assertThat(connectFuture.await(10000), is(true));
assertThat(connectFuture.cause(), sameInstance((Throwable) exception)); assertThat(connectFuture.cause(), sameInstance((Throwable) exception));
assertThat(connectFuture.channel(), is(nullValue())); assertThat(connectFuture.channel(), is(not(nullValue())));
} }
private static final class DelayedEventLoopGroup extends DefaultEventLoop { private static final class DelayedEventLoopGroup extends DefaultEventLoop {

View File

@ -0,0 +1,38 @@
/*
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.channel;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.concurrent.ImmediateEventExecutor;
import org.junit.Test;
public class DefaultChannelPromiseTest {
@Test(expected = NullPointerException.class)
public void testNullChannel() {
new DefaultChannelPromise(null);
}
@Test(expected = NullPointerException.class)
public void testChannelWithNullExecutor() {
new DefaultChannelPromise(new EmbeddedChannel(), null);
}
@Test(expected = NullPointerException.class)
public void testNullChannelWithExecutor() {
new DefaultChannelPromise(null, ImmediateEventExecutor.INSTANCE);
}
}