Skip to content

Commit

Permalink
Fix NioEventLoopTest#testChannelsRegistered flakiness (netty#9650)
Browse files Browse the repository at this point in the history
Motivation

NioEventLoopTest#testChannelsRegistered() fails intermittently due to
use of SingleThreadEventLoop#channelsRegistered() which is not
threadsafe and unreliable when called from outside the event loop.

Modifications

Add static registeredChannels method to NioEventLoopTest and
AbstractSingleThreadEventLoopTest to call from the tests via event loop
instead of directly.

Result

Hopefully fewer test failures
  • Loading branch information
njhill authored and normanmaurer committed Oct 9, 2019
1 parent 271d8de commit 99a4cec
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeUnit;
Expand All @@ -42,36 +43,46 @@
public abstract class AbstractSingleThreadEventLoopTest {

@Test
public void testChannelsRegistered() {
public void testChannelsRegistered() throws Exception {
EventLoopGroup group = newEventLoopGroup();
final SingleThreadEventLoop loop = (SingleThreadEventLoop) group.next();

try {
final Channel ch1 = newChannel();
final Channel ch2 = newChannel();

int rc = loop.registeredChannels();
int rc = registeredChannels(loop);
boolean channelCountSupported = rc != -1;

if (channelCountSupported) {
assertEquals(0, loop.registeredChannels());
assertEquals(0, registeredChannels(loop));
}

assertTrue(loop.register(ch1).syncUninterruptibly().isSuccess());
assertTrue(loop.register(ch2).syncUninterruptibly().isSuccess());
if (channelCountSupported) {
assertEquals(2, loop.registeredChannels());
assertEquals(2, registeredChannels(loop));
}

assertTrue(ch1.deregister().syncUninterruptibly().isSuccess());
if (channelCountSupported) {
assertEquals(1, loop.registeredChannels());
assertEquals(1, registeredChannels(loop));
}
} finally {
group.shutdownGracefully();
}
}

// Only reliable if run from event loop
private static int registeredChannels(final SingleThreadEventLoop loop) throws Exception {
return loop.submit(new Callable<Integer>() {
@Override
public Integer call() {
return loop.registeredChannels();
}
}).get(1, TimeUnit.SECONDS);
}

@Test
@SuppressWarnings("deprecation")
public void shutdownBeforeStart() throws Exception {
Expand Down
20 changes: 16 additions & 4 deletions transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.netty.channel.EventLoopTaskQueueFactory;
import io.netty.channel.SelectStrategy;
import io.netty.channel.SelectStrategyFactory;
import io.netty.channel.SingleThreadEventLoop;
import io.netty.channel.socket.ServerSocketChannel;
import io.netty.channel.socket.nio.NioServerSocketChannel;
import io.netty.util.IntSupplier;
Expand All @@ -42,6 +43,7 @@
import java.nio.channels.SocketChannel;
import java.nio.channels.spi.SelectorProvider;
import java.util.Queue;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.RejectedExecutionException;
Expand Down Expand Up @@ -269,27 +271,37 @@ public int calculateStrategy(IntSupplier selectSupplier, boolean hasTasks) throw

@Ignore
@Test
public void testChannelsRegistered() {
public void testChannelsRegistered() throws Exception {
NioEventLoopGroup group = new NioEventLoopGroup(1);
final NioEventLoop loop = (NioEventLoop) group.next();

try {
final Channel ch1 = new NioServerSocketChannel();
final Channel ch2 = new NioServerSocketChannel();

assertEquals(0, loop.registeredChannels());
assertEquals(0, registeredChannels(loop));

assertTrue(loop.register(ch1).syncUninterruptibly().isSuccess());
assertTrue(loop.register(ch2).syncUninterruptibly().isSuccess());
assertEquals(2, loop.registeredChannels());
assertEquals(2, registeredChannels(loop));

assertTrue(ch1.deregister().syncUninterruptibly().isSuccess());
assertEquals(1, loop.registeredChannels());
assertEquals(1, registeredChannels(loop));
} finally {
group.shutdownGracefully();
}
}

// Only reliable if run from event loop
private static int registeredChannels(final SingleThreadEventLoop loop) throws Exception {
return loop.submit(new Callable<Integer>() {
@Override
public Integer call() {
return loop.registeredChannels();
}
}).get(1, TimeUnit.SECONDS);
}

@Test
public void testCustomQueue() {
final AtomicBoolean called = new AtomicBoolean();
Expand Down

0 comments on commit 99a4cec

Please sign in to comment.