From b3d3269b8c8f34e252c1db2caa005e9adc8696fa Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 31 Aug 2021 16:05:21 +0200 Subject: [PATCH] Avoid overspecifying ScheduledFuture when Future will do (#11636) Motivation: Overspecifying interfaces can increase coupling and make refactoring harder down the line. Modification: Places that were specifying fields, variables, and return types as ScheduledFuture, but did not use any features specific to ScheduledFuture, have been changed to specify Future instead. Result: Cleaner code. --- .../websocketx/WebSocketProtocolHandler.java | 4 +- .../codec/http2/Http2ConnectionHandler.java | 4 +- .../io/netty/handler/proxy/ProxyHandler.java | 3 +- .../java/io/netty/handler/ssl/SslHandler.java | 6 +-- .../handler/timeout/IdleStateHandler.java | 10 ++--- .../handler/timeout/WriteTimeoutHandler.java | 4 +- .../handler/timeout/IdleStateHandlerTest.java | 4 +- .../epoll/EpollSocketChannelBenchmark.java | 7 ++-- .../netty/resolver/dns/DnsQueryContext.java | 5 +-- .../channel/epoll/AbstractEpollChannel.java | 6 +-- .../channel/kqueue/AbstractKQueueChannel.java | 4 +- .../netty/channel/nio/AbstractNioChannel.java | 6 +-- .../channel/embedded/EmbeddedChannelTest.java | 42 +++++++++---------- 13 files changed, 51 insertions(+), 54 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java index 930edcc823..e7f87c0f9e 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketProtocolHandler.java @@ -23,8 +23,8 @@ import io.netty.channel.ChannelOutboundHandler; import io.netty.channel.ChannelPromise; import io.netty.handler.codec.MessageToMessageDecoder; import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; import io.netty.util.concurrent.PromiseNotifier; -import io.netty.util.concurrent.ScheduledFuture; import java.net.SocketAddress; import java.nio.channels.ClosedChannelException; @@ -128,7 +128,7 @@ abstract class WebSocketProtocolHandler extends MessageToMessageDecoder timeoutTask = ctx.executor().schedule(new Runnable() { + final Future timeoutTask = ctx.executor().schedule(new Runnable() { @Override public void run() { if (!closeSent.isDone()) { diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java index 9bd3b51f46..bf56132b9f 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java @@ -27,7 +27,7 @@ import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http2.Http2Exception.CompositeStreamException; import io.netty.handler.codec.http2.Http2Exception.StreamException; import io.netty.util.CharsetUtil; -import io.netty.util.concurrent.ScheduledFuture; +import io.netty.util.concurrent.Future; import io.netty.util.internal.UnstableApi; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -943,7 +943,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http private static final class ClosingChannelFutureListener implements ChannelFutureListener { private final ChannelHandlerContext ctx; private final ChannelPromise promise; - private final ScheduledFuture timeoutTask; + private final Future timeoutTask; private boolean closed; ClosingChannelFutureListener(ChannelHandlerContext ctx, ChannelPromise promise) { diff --git a/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java b/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java index cb2219e99d..dd17de2dc2 100644 --- a/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java +++ b/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java @@ -27,7 +27,6 @@ import io.netty.util.ReferenceCountUtil; import io.netty.util.concurrent.DefaultPromise; import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.Future; -import io.netty.util.concurrent.ScheduledFuture; import io.netty.util.internal.ObjectUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -60,7 +59,7 @@ public abstract class ProxyHandler extends ChannelDuplexHandler { private boolean suppressChannelReadComplete; private boolean flushedPrematurely; private final LazyChannelPromise connectPromise = new LazyChannelPromise(); - private ScheduledFuture connectTimeoutFuture; + private Future connectTimeoutFuture; private final ChannelFutureListener writeListener = new ChannelFutureListener() { @Override public void operationComplete(ChannelFuture future) throws Exception { diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 4f0a228159..8bf89a4276 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -2102,7 +2102,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH return; } - final ScheduledFuture timeoutFuture = ctx.executor().schedule(new Runnable() { + final Future timeoutFuture = ctx.executor().schedule(new Runnable() { @Override public void run() { if (localHandshakePromise.isDone()) { @@ -2153,7 +2153,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH return; } - final ScheduledFuture timeoutFuture; + final Future timeoutFuture; if (!flushFuture.isDone()) { long closeNotifyTimeout = closeNotifyFlushTimeoutMillis; if (closeNotifyTimeout > 0) { @@ -2189,7 +2189,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH // See https://github.com/netty/netty/issues/2358 addCloseListener(ctx.close(ctx.newPromise()), promise); } else { - final ScheduledFuture closeNotifyReadTimeoutFuture; + final Future closeNotifyReadTimeoutFuture; if (!sslClosePromise.isDone()) { closeNotifyReadTimeoutFuture = ctx.executor().schedule(new Runnable() { diff --git a/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java b/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java index db3805e7a2..f1648cb69e 100644 --- a/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java +++ b/handler/src/main/java/io/netty/handler/timeout/IdleStateHandler.java @@ -25,9 +25,9 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInitializer; import io.netty.channel.ChannelOutboundBuffer; import io.netty.channel.ChannelPromise; +import io.netty.util.concurrent.Future; import io.netty.util.internal.ObjectUtil; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; /** @@ -113,15 +113,15 @@ public class IdleStateHandler extends ChannelDuplexHandler { private final long writerIdleTimeNanos; private final long allIdleTimeNanos; - private ScheduledFuture readerIdleTimeout; + private Future readerIdleTimeout; private long lastReadTime; private boolean firstReaderIdleEvent = true; - private ScheduledFuture writerIdleTimeout; + private Future writerIdleTimeout; private long lastWriteTime; private boolean firstWriterIdleEvent = true; - private ScheduledFuture allIdleTimeout; + private Future allIdleTimeout; private boolean firstAllIdleEvent = true; private byte state; // 0 - none, 1 - initialized, 2 - destroyed @@ -344,7 +344,7 @@ public class IdleStateHandler extends ChannelDuplexHandler { /** * This method is visible for testing! */ - ScheduledFuture schedule(ChannelHandlerContext ctx, Runnable task, long delay, TimeUnit unit) { + Future schedule(ChannelHandlerContext ctx, Runnable task, long delay, TimeUnit unit) { return ctx.executor().schedule(task, delay, unit); } diff --git a/handler/src/main/java/io/netty/handler/timeout/WriteTimeoutHandler.java b/handler/src/main/java/io/netty/handler/timeout/WriteTimeoutHandler.java index 0968f3322e..be2c0c2bcc 100644 --- a/handler/src/main/java/io/netty/handler/timeout/WriteTimeoutHandler.java +++ b/handler/src/main/java/io/netty/handler/timeout/WriteTimeoutHandler.java @@ -24,9 +24,9 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInitializer; import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelPromise; +import io.netty.util.concurrent.Future; import io.netty.util.internal.ObjectUtil; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; /** @@ -192,7 +192,7 @@ public class WriteTimeoutHandler extends ChannelOutboundHandlerAdapter { WriteTimeoutTask prev; WriteTimeoutTask next; - ScheduledFuture scheduledFuture; + Future scheduledFuture; WriteTimeoutTask(ChannelHandlerContext ctx, ChannelPromise promise) { this.ctx = ctx; diff --git a/handler/src/test/java/io/netty/handler/timeout/IdleStateHandlerTest.java b/handler/src/test/java/io/netty/handler/timeout/IdleStateHandlerTest.java index 49f06bc708..2256487fb7 100644 --- a/handler/src/test/java/io/netty/handler/timeout/IdleStateHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/timeout/IdleStateHandlerTest.java @@ -22,11 +22,11 @@ import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelOutboundBuffer; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; import org.junit.jupiter.api.Test; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; @@ -384,7 +384,7 @@ public class IdleStateHandlerTest { } @Override - ScheduledFuture schedule(ChannelHandlerContext ctx, Runnable task, long delay, TimeUnit unit) { + Future schedule(ChannelHandlerContext ctx, Runnable task, long delay, TimeUnit unit) { this.task = task; this.delayInNanos = unit.toNanos(delay); return null; diff --git a/microbench/src/main/java/io/netty/microbench/channel/epoll/EpollSocketChannelBenchmark.java b/microbench/src/main/java/io/netty/microbench/channel/epoll/EpollSocketChannelBenchmark.java index 1069998c63..268c08ed53 100644 --- a/microbench/src/main/java/io/netty/microbench/channel/epoll/EpollSocketChannelBenchmark.java +++ b/microbench/src/main/java/io/netty/microbench/channel/epoll/EpollSocketChannelBenchmark.java @@ -27,13 +27,14 @@ import io.netty.channel.epoll.EpollEventLoopGroup; import io.netty.channel.epoll.EpollServerSocketChannel; import io.netty.channel.epoll.EpollSocketChannel; import io.netty.microbench.util.AbstractMicrobenchmark; -import io.netty.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; +import io.netty.util.concurrent.Future; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.GroupThreads; import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.TearDown; +import java.util.concurrent.TimeUnit; + public class EpollSocketChannelBenchmark extends AbstractMicrobenchmark { private static final Runnable runnable = new Runnable() { @Override @@ -44,7 +45,7 @@ public class EpollSocketChannelBenchmark extends AbstractMicrobenchmark { private Channel serverChan; private Channel chan; private ByteBuf abyte; - private ScheduledFuture future; + private Future future; @Setup public void setup() throws Exception { diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java index 94cfa99d48..a40d4afeb0 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsQueryContext.java @@ -30,7 +30,6 @@ import io.netty.util.concurrent.Future; import io.netty.util.concurrent.FutureListener; import io.netty.util.concurrent.GenericFutureListener; import io.netty.util.concurrent.Promise; -import io.netty.util.concurrent.ScheduledFuture; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -52,7 +51,7 @@ abstract class DnsQueryContext implements FutureListener timeoutFuture; + private volatile Future timeoutFuture; DnsQueryContext(DnsNameResolver parent, InetSocketAddress nameServerAddr, @@ -228,7 +227,7 @@ abstract class DnsQueryContext implements FutureListener> future) { // Cancel the timeout task. - final ScheduledFuture timeoutFuture = this.timeoutFuture; + Future timeoutFuture = this.timeoutFuture; if (timeoutFuture != null) { this.timeoutFuture = null; timeoutFuture.cancel(false); diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java index 3589ac818f..baaee987e4 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java @@ -39,6 +39,7 @@ import io.netty.channel.unix.IovArray; import io.netty.channel.unix.Socket; import io.netty.channel.unix.UnixChannel; import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; import java.io.IOException; import java.net.InetSocketAddress; @@ -49,7 +50,6 @@ import java.nio.channels.ClosedChannelException; import java.nio.channels.ConnectionPendingException; import java.nio.channels.NotYetConnectedException; import java.nio.channels.UnresolvedAddressException; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import static io.netty.channel.internal.ChannelUtils.WRITE_STATUS_SNDBUF_FULL; @@ -64,7 +64,7 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann * connection attempts will fail. */ private ChannelPromise connectPromise; - private ScheduledFuture connectTimeoutFuture; + private Future connectTimeoutFuture; private SocketAddress requestedRemoteAddress; private volatile SocketAddress local; @@ -160,7 +160,7 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann connectPromise = null; } - ScheduledFuture future = connectTimeoutFuture; + Future future = connectTimeoutFuture; if (future != null) { future.cancel(false); connectTimeoutFuture = null; diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/AbstractKQueueChannel.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/AbstractKQueueChannel.java index 80b27858a3..0096a60124 100644 --- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/AbstractKQueueChannel.java +++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/AbstractKQueueChannel.java @@ -37,6 +37,7 @@ import io.netty.channel.socket.SocketChannelConfig; import io.netty.channel.unix.FileDescriptor; import io.netty.channel.unix.UnixChannel; import io.netty.util.ReferenceCountUtil; +import io.netty.util.concurrent.Future; import java.io.IOException; import java.net.ConnectException; @@ -47,7 +48,6 @@ import java.nio.channels.AlreadyConnectedException; import java.nio.channels.ConnectionPendingException; import java.nio.channels.NotYetConnectedException; import java.nio.channels.UnresolvedAddressException; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import static io.netty.channel.internal.ChannelUtils.WRITE_STATUS_SNDBUF_FULL; @@ -61,7 +61,7 @@ abstract class AbstractKQueueChannel extends AbstractChannel implements UnixChan * connection attempts will fail. */ private ChannelPromise connectPromise; - private ScheduledFuture connectTimeoutFuture; + private Future connectTimeoutFuture; private SocketAddress requestedRemoteAddress; final BsdSocket socket; diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java index baa08f218d..7563e4748d 100644 --- a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java +++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java @@ -29,6 +29,7 @@ import io.netty.channel.ConnectTimeoutException; import io.netty.channel.EventLoop; import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCounted; +import io.netty.util.concurrent.Future; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -39,7 +40,6 @@ import java.nio.channels.ClosedChannelException; import java.nio.channels.ConnectionPendingException; import java.nio.channels.SelectableChannel; import java.nio.channels.SelectionKey; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; /** @@ -66,7 +66,7 @@ public abstract class AbstractNioChannel extends AbstractChannel { * connection attempts will fail. */ private ChannelPromise connectPromise; - private ScheduledFuture connectTimeoutFuture; + private Future connectTimeoutFuture; private SocketAddress requestedRemoteAddress; /** @@ -503,7 +503,7 @@ public abstract class AbstractNioChannel extends AbstractChannel { connectPromise = null; } - ScheduledFuture future = connectTimeoutFuture; + Future future = connectTimeoutFuture; if (future != null) { future.cancel(false); connectTimeoutFuture = null; diff --git a/transport/src/test/java/io/netty/channel/embedded/EmbeddedChannelTest.java b/transport/src/test/java/io/netty/channel/embedded/EmbeddedChannelTest.java index 7f43a6e1db..476ff2b0e8 100644 --- a/transport/src/test/java/io/netty/channel/embedded/EmbeddedChannelTest.java +++ b/transport/src/test/java/io/netty/channel/embedded/EmbeddedChannelTest.java @@ -15,25 +15,6 @@ */ package io.netty.channel.embedded; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - -import java.nio.channels.ClosedChannelException; -import java.util.ArrayDeque; -import java.util.Queue; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Timeout; - import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.Channel; @@ -51,7 +32,24 @@ import io.netty.channel.ChannelPromise; import io.netty.util.ReferenceCountUtil; import io.netty.util.concurrent.Future; import io.netty.util.concurrent.FutureListener; -import io.netty.util.concurrent.ScheduledFuture; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.nio.channels.ClosedChannelException; +import java.util.ArrayDeque; +import java.util.Queue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class EmbeddedChannelTest { @@ -134,7 +132,7 @@ public class EmbeddedChannelTest { public void testScheduling() throws Exception { EmbeddedChannel ch = new EmbeddedChannel(new ChannelInboundHandlerAdapter()); final CountDownLatch latch = new CountDownLatch(2); - ScheduledFuture future = ch.eventLoop().schedule(new Runnable() { + Future future = ch.eventLoop().schedule(new Runnable() { @Override public void run() { latch.countDown(); @@ -158,7 +156,7 @@ public class EmbeddedChannelTest { @Test public void testScheduledCancelled() throws Exception { EmbeddedChannel ch = new EmbeddedChannel(new ChannelInboundHandlerAdapter()); - ScheduledFuture future = ch.eventLoop().schedule(new Runnable() { + Future future = ch.eventLoop().schedule(new Runnable() { @Override public void run() { } }, 1, TimeUnit.DAYS);