Ensure we create a copy of the attributes and options when bootstrap a channel (#10965)

Motivation:

We need to ensure we copy the attributes and options when bootstrap the channel as otherwise we may change the underlying Entry.
This is similar to what was reported in https://github.com/netty/netty-incubator-codec-quic/issues/152.

Modifications:

- Do a copy and re-use methods
- Add unit tests

Result:

Don't affect attributes / options of channels that are already bootstrapped
This commit is contained in:
Norman Maurer 2021-01-26 19:26:20 +01:00 committed by GitHub
parent 305cb1719a
commit 1e87c711b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 44 additions and 10 deletions

View File

@ -52,9 +52,9 @@ import java.util.concurrent.ConcurrentHashMap;
*/ */
public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C extends Channel> implements Cloneable { public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C extends Channel> implements Cloneable {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
static final Map.Entry<ChannelOption<?>, Object>[] EMPTY_OPTION_ARRAY = new Map.Entry[0]; private static final Map.Entry<ChannelOption<?>, Object>[] EMPTY_OPTION_ARRAY = new Map.Entry[0];
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
static final Map.Entry<AttributeKey<?>, Object>[] EMPTY_ATTRIBUTE_ARRAY = new Map.Entry[0]; private static final Map.Entry<AttributeKey<?>, Object>[] EMPTY_ATTRIBUTE_ARRAY = new Map.Entry[0];
volatile EventLoopGroup group; volatile EventLoopGroup group;
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
@ -386,11 +386,23 @@ public abstract class AbstractBootstrap<B extends AbstractBootstrap<B, C>, C ext
public abstract AbstractBootstrapConfig<B, C> config(); public abstract AbstractBootstrapConfig<B, C> config();
final Map.Entry<ChannelOption<?>, Object>[] newOptionsArray() { final Map.Entry<ChannelOption<?>, Object>[] newOptionsArray() {
return newOptionsArray(options);
}
static Map.Entry<ChannelOption<?>, Object>[] newOptionsArray(Map<ChannelOption<?>, Object> options) {
synchronized (options) { synchronized (options) {
return options.entrySet().toArray(EMPTY_OPTION_ARRAY); return new LinkedHashMap<ChannelOption<?>, Object>(options).entrySet().toArray(EMPTY_OPTION_ARRAY);
} }
} }
final Map.Entry<AttributeKey<?>, Object>[] newAttributesArray() {
return newAttributesArray(attrs0());
}
static Map.Entry<AttributeKey<?>, Object>[] newAttributesArray(Map<AttributeKey<?>, Object> attributes) {
return attributes.entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY);
}
final Map<ChannelOption<?>, Object> options0() { final Map<ChannelOption<?>, Object> options0() {
return options; return options;
} }

View File

@ -263,7 +263,7 @@ public class Bootstrap extends AbstractBootstrap<Bootstrap, Channel> {
p.addLast(config.handler()); p.addLast(config.handler());
setChannelOptions(channel, newOptionsArray(), logger); setChannelOptions(channel, newOptionsArray(), logger);
setAttributes(channel, attrs0().entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY)); setAttributes(channel, newAttributesArray());
} }
@Override @Override

View File

@ -130,17 +130,14 @@ public class ServerBootstrap extends AbstractBootstrap<ServerBootstrap, ServerCh
@Override @Override
void init(Channel channel) { void init(Channel channel) {
setChannelOptions(channel, newOptionsArray(), logger); setChannelOptions(channel, newOptionsArray(), logger);
setAttributes(channel, attrs0().entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY)); setAttributes(channel, newAttributesArray());
ChannelPipeline p = channel.pipeline(); ChannelPipeline p = channel.pipeline();
final EventLoopGroup currentChildGroup = childGroup; final EventLoopGroup currentChildGroup = childGroup;
final ChannelHandler currentChildHandler = childHandler; final ChannelHandler currentChildHandler = childHandler;
final Entry<ChannelOption<?>, Object>[] currentChildOptions; final Entry<ChannelOption<?>, Object>[] currentChildOptions = newOptionsArray(childOptions);
synchronized (childOptions) { final Entry<AttributeKey<?>, Object>[] currentChildAttrs = newAttributesArray(childAttrs);
currentChildOptions = childOptions.entrySet().toArray(EMPTY_OPTION_ARRAY);
}
final Entry<AttributeKey<?>, Object>[] currentChildAttrs = childAttrs.entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY);
p.addLast(new ChannelInitializer<Channel>() { p.addLast(new ChannelInitializer<Channel>() {
@Override @Override

View File

@ -38,6 +38,7 @@ import io.netty.channel.local.LocalServerChannel;
import io.netty.resolver.AddressResolver; import io.netty.resolver.AddressResolver;
import io.netty.resolver.AddressResolverGroup; import io.netty.resolver.AddressResolverGroup;
import io.netty.resolver.AbstractAddressResolver; import io.netty.resolver.AbstractAddressResolver;
import io.netty.util.AttributeKey;
import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.Future; import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.Promise; import io.netty.util.concurrent.Promise;
@ -51,6 +52,8 @@ import java.net.UnknownHostException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.BlockingQueue; import java.util.concurrent.BlockingQueue;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.LinkedBlockingQueue;
@ -73,6 +76,28 @@ public class BootstrapTest {
groupB.terminationFuture().syncUninterruptibly(); groupB.terminationFuture().syncUninterruptibly();
} }
@Test
public void testOptionsCopied() {
final Bootstrap bootstrapA = new Bootstrap();
bootstrapA.option(ChannelOption.AUTO_READ, true);
Map.Entry<ChannelOption<?>, Object>[] channelOptions = bootstrapA.newOptionsArray();
bootstrapA.option(ChannelOption.AUTO_READ, false);
assertEquals(ChannelOption.AUTO_READ, channelOptions[0].getKey());
assertEquals(true, channelOptions[0].getValue());
}
@Test
public void testAttributesCopied() {
AttributeKey<String> key = AttributeKey.valueOf(UUID.randomUUID().toString());
String value = "value";
final Bootstrap bootstrapA = new Bootstrap();
bootstrapA.attr(key, value);
Map.Entry<AttributeKey<?>, Object>[] attributesArray = bootstrapA.newAttributesArray();
bootstrapA.attr(key, "value2");
assertEquals(key, attributesArray[0].getKey());
assertEquals(value, attributesArray[0].getValue());
}
@Test(timeout = 10000) @Test(timeout = 10000)
public void testBindDeadLock() throws Exception { public void testBindDeadLock() throws Exception {
final Bootstrap bootstrapA = new Bootstrap(); final Bootstrap bootstrapA = new Bootstrap();