DnsAddressResolverGroup should respect configured EventLoop (#10479)

Motivation:

DnsAddressResolverGroup allows to be constructed with a DnsNameResolverBuilder and so should respect its configured EventLoop.

Modifications:

- Correctly respect the configured EventLoop
- Ensure there are no thread-issues by calling copy()
- Add unit tests

Result:

Fixes https://github.com/netty/netty/issues/10460
This commit is contained in:
Norman Maurer 2020-08-13 20:30:14 +02:00
parent 03b6be774d
commit cc33da49aa
3 changed files with 75 additions and 5 deletions

View File

@ -50,14 +50,14 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
public DnsAddressResolverGroup( public DnsAddressResolverGroup(
Class<? extends DatagramChannel> channelType, Class<? extends DatagramChannel> channelType,
DnsServerAddressStreamProvider nameServerProvider) { DnsServerAddressStreamProvider nameServerProvider) {
this(new DnsNameResolverBuilder()); this.dnsResolverBuilder = new DnsNameResolverBuilder();
dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider); dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider);
} }
public DnsAddressResolverGroup( public DnsAddressResolverGroup(
ChannelFactory<? extends DatagramChannel> channelFactory, ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddressStreamProvider nameServerProvider) { DnsServerAddressStreamProvider nameServerProvider) {
this(new DnsNameResolverBuilder()); this.dnsResolverBuilder = new DnsNameResolverBuilder();
dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider); dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider);
} }
@ -72,7 +72,8 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
// we don't really need to pass channelFactory and nameServerProvider separately, // we don't really need to pass channelFactory and nameServerProvider separately,
// but still keep this to ensure backward compatibility with (potentially) override methods // but still keep this to ensure backward compatibility with (potentially) override methods
return newResolver((EventLoop) executor, EventLoop loop = dnsResolverBuilder.eventLoop;
return newResolver(loop == null ? (EventLoop) executor : loop,
dnsResolverBuilder.channelFactory(), dnsResolverBuilder.channelFactory(),
dnsResolverBuilder.nameServerProvider()); dnsResolverBuilder.nameServerProvider());
} }
@ -102,9 +103,11 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
ChannelFactory<? extends DatagramChannel> channelFactory, ChannelFactory<? extends DatagramChannel> channelFactory,
DnsServerAddressStreamProvider nameServerProvider) DnsServerAddressStreamProvider nameServerProvider)
throws Exception { throws Exception {
DnsNameResolverBuilder builder = dnsResolverBuilder.copy();
// once again, channelFactory and nameServerProvider are most probably set in builder already, // once again, channelFactory and nameServerProvider are most probably set in builder already,
// but I do reassign them again to avoid corner cases with override methods // but I do reassign them again to avoid corner cases with override methods
return dnsResolverBuilder.eventLoop(eventLoop) return builder.eventLoop(eventLoop)
.channelFactory(channelFactory) .channelFactory(channelFactory)
.nameServerProvider(nameServerProvider) .nameServerProvider(nameServerProvider)
.build(); .build();

View File

@ -36,7 +36,7 @@ import static java.util.Objects.requireNonNull;
* A {@link DnsNameResolver} builder. * A {@link DnsNameResolver} builder.
*/ */
public final class DnsNameResolverBuilder { public final class DnsNameResolverBuilder {
private EventLoop eventLoop; volatile EventLoop eventLoop;
private ChannelFactory<? extends DatagramChannel> channelFactory; private ChannelFactory<? extends DatagramChannel> channelFactory;
private ChannelFactory<? extends SocketChannel> socketChannelFactory; private ChannelFactory<? extends SocketChannel> socketChannelFactory;
private DnsCache resolveCache; private DnsCache resolveCache;

View File

@ -0,0 +1,67 @@
/*
* Copyright 2020 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.resolver.dns;
import io.netty.channel.EventLoop;
import io.netty.channel.MultithreadEventLoopGroup;
import io.netty.channel.local.LocalHandler;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.nio.NioDatagramChannel;
import io.netty.resolver.AddressResolver;
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.FutureListener;
import io.netty.util.concurrent.Promise;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
import java.net.SocketAddress;
import java.nio.channels.UnsupportedAddressTypeException;
public class DnsAddressResolverGroupTest {
@Test
public void testUseConfiguredEventLoop() throws InterruptedException {
NioEventLoopGroup group = new NioEventLoopGroup(1);
final EventLoop loop = group.next();
MultithreadEventLoopGroup defaultEventLoopGroup = new MultithreadEventLoopGroup(1, LocalHandler.newFactory());
DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
.eventLoop(loop).channelType(NioDatagramChannel.class);
DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(builder);
try {
final Promise<?> promise = loop.newPromise();
AddressResolver<?> resolver = resolverGroup.getResolver(defaultEventLoopGroup.next());
resolver.resolve(new SocketAddress() {
private static final long serialVersionUID = 3169703458729818468L;
}).addListener(new FutureListener<Object>() {
@Override
public void operationComplete(Future<Object> future) {
try {
Assert.assertThat(future.cause(), Matchers.instanceOf(UnsupportedAddressTypeException.class));
Assert.assertTrue(loop.inEventLoop());
promise.setSuccess(null);
} catch (Throwable cause) {
promise.setFailure(cause);
}
}
}).await();
promise.sync();
} finally {
resolverGroup.close();
group.shutdownGracefully();
defaultEventLoopGroup.shutdownGracefully();
}
}
}