diff --git a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java index 795aeba907c..6585cd84f54 100644 --- a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java +++ b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java @@ -150,7 +150,7 @@ public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) if (delay < 0) { delay = 0; } - validateScheduled(delay, unit); + validateScheduled0(delay, unit); return schedule(new ScheduledFutureTask( this, command, null, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay)))); @@ -163,7 +163,7 @@ public ScheduledFuture schedule(Callable callable, long delay, TimeUni if (delay < 0) { delay = 0; } - validateScheduled(delay, unit); + validateScheduled0(delay, unit); return schedule(new ScheduledFutureTask( this, callable, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay)))); @@ -181,8 +181,8 @@ public ScheduledFuture scheduleAtFixedRate(Runnable command, long initialDela throw new IllegalArgumentException( String.format("period: %d (expected: > 0)", period)); } - validateScheduled(initialDelay, unit); - validateScheduled(period, unit); + validateScheduled0(initialDelay, unit); + validateScheduled0(period, unit); return schedule(new ScheduledFutureTask( this, Executors.callable(command, null), @@ -202,17 +202,25 @@ public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialD String.format("delay: %d (expected: > 0)", delay)); } - validateScheduled(initialDelay, unit); - validateScheduled(delay, unit); + validateScheduled0(initialDelay, unit); + validateScheduled0(delay, unit); return schedule(new ScheduledFutureTask( this, Executors.callable(command, null), ScheduledFutureTask.deadlineNanos(unit.toNanos(initialDelay)), -unit.toNanos(delay))); } + @SuppressWarnings("deprecation") + private void validateScheduled0(long amount, TimeUnit unit) { + validateScheduled(amount, unit); + } + /** * Sub-classes may override this to restrict the maximal amount of time someone can use to schedule a task. + * + * @deprecated will be removed in the future. */ + @Deprecated protected void validateScheduled(long amount, TimeUnit unit) { // NOOP } diff --git a/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java b/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java index 6043d6ffec0..1eaa7b92762 100644 --- a/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java +++ b/common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java @@ -35,7 +35,9 @@ static long nanoTime() { } static long deadlineNanos(long delay) { - return nanoTime() + delay; + long deadlineNanos = nanoTime() + delay; + // Guard against overflow + return deadlineNanos < 0 ? Long.MAX_VALUE : deadlineNanos; } private final long id = nextTaskId.getAndIncrement(); diff --git a/common/src/test/java/io/netty/util/concurrent/ScheduledFutureTaskTest.java b/common/src/test/java/io/netty/util/concurrent/ScheduledFutureTaskTest.java new file mode 100644 index 00000000000..1c12692ff31 --- /dev/null +++ b/common/src/test/java/io/netty/util/concurrent/ScheduledFutureTaskTest.java @@ -0,0 +1,27 @@ +/* + * Copyright 2018 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.util.concurrent; + +import org.junit.Assert; +import org.junit.Test; + +public class ScheduledFutureTaskTest { + + @Test + public void testDeadlineNanosNotOverflow() { + Assert.assertEquals(Long.MAX_VALUE, ScheduledFutureTask.deadlineNanos(Long.MAX_VALUE)); + } +} diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java index cf55ef67ec5..913f5edf429 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java @@ -37,7 +37,6 @@ import java.util.Queue; import java.util.concurrent.Callable; import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import static java.lang.Math.min; @@ -80,7 +79,7 @@ public Integer call() throws Exception { private volatile int ioRatio = 50; // See http://man7.org/linux/man-pages/man2/timerfd_create.2.html. - static final long MAX_SCHEDULED_DAYS = TimeUnit.SECONDS.toDays(999999999); + private static final long MAX_SCHEDULED_TIMERFD_NS = 999999999; EpollEventLoop(EventLoopGroup parent, Executor executor, int maxEvents, SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler) { @@ -237,7 +236,7 @@ private int epollWait(boolean oldWakeup) throws IOException { long totalDelay = delayNanos(System.nanoTime()); int delaySeconds = (int) min(totalDelay / 1000000000L, Integer.MAX_VALUE); return Native.epollWait(epollFd, events, timerFd, delaySeconds, - (int) min(totalDelay - delaySeconds * 1000000000L, Integer.MAX_VALUE)); + (int) min(MAX_SCHEDULED_TIMERFD_NS, totalDelay - delaySeconds * 1000000000L)); } private int epollWaitNow() throws IOException { @@ -453,12 +452,4 @@ protected void cleanup() { events.free(); } } - - @Override - protected void validateScheduled(long amount, TimeUnit unit) { - long days = unit.toDays(amount); - if (days > MAX_SCHEDULED_DAYS) { - throw new IllegalArgumentException("days: " + days + " (expected: < " + MAX_SCHEDULED_DAYS + ')'); - } - } } diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java index 0fe824b334d..ebf529f7328 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java @@ -24,32 +24,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class EpollEventLoopTest { - @Test(timeout = 5000L) - public void testScheduleBigDelayOverMax() { - EventLoopGroup group = new EpollEventLoopGroup(1); - - final EventLoop el = group.next(); - try { - el.schedule(new Runnable() { - @Override - public void run() { - // NOOP - } - }, Integer.MAX_VALUE, TimeUnit.DAYS); - fail(); - } catch (IllegalArgumentException expected) { - // expected - } - - group.shutdownGracefully(); - } - @Test - public void testScheduleBigDelay() { + public void testScheduleBigDelayNotOverflow() { EventLoopGroup group = new EpollEventLoopGroup(1); final EventLoop el = group.next(); @@ -58,7 +37,7 @@ public void testScheduleBigDelay() { public void run() { // NOOP } - }, EpollEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS); + }, Long.MAX_VALUE, TimeUnit.MILLISECONDS); assertFalse(future.awaitUninterruptibly(1000)); assertTrue(future.cancel(true)); diff --git a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java index 3badcea96c7..5af59ba912c 100644 --- a/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java +++ b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java @@ -33,7 +33,6 @@ import java.util.Queue; import java.util.concurrent.Callable; import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import static io.netty.channel.kqueue.KQueueEventArray.deleteGlobalRefs; @@ -77,8 +76,6 @@ public Integer call() throws Exception { private volatile int wakenUp; private volatile int ioRatio = 50; - static final long MAX_SCHEDULED_DAYS = 365 * 3; - KQueueEventLoop(EventLoopGroup parent, Executor executor, int maxEvents, SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler) { super(parent, executor, false, DEFAULT_MAX_PENDING_TASKS, rejectedExecutionHandler); @@ -370,12 +367,4 @@ private static void handleLoopException(Throwable t) { // Ignore. } } - - @Override - protected void validateScheduled(long amount, TimeUnit unit) { - long days = unit.toDays(amount); - if (days > MAX_SCHEDULED_DAYS) { - throw new IllegalArgumentException("days: " + days + " (expected: < " + MAX_SCHEDULED_DAYS + ')'); - } - } } diff --git a/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueEventLoopTest.java b/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueEventLoopTest.java index c7ad56e37ab..0d441559994 100644 --- a/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueEventLoopTest.java +++ b/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueEventLoopTest.java @@ -24,32 +24,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; public class KQueueEventLoopTest { - @Test(timeout = 5000L) - public void testScheduleBigDelayOverMax() { - EventLoopGroup group = new KQueueEventLoopGroup(1); - - final EventLoop el = group.next(); - try { - el.schedule(new Runnable() { - @Override - public void run() { - // NOOP - } - }, Integer.MAX_VALUE, TimeUnit.DAYS); - fail(); - } catch (IllegalArgumentException expected) { - // expected - } - - group.shutdownGracefully(); - } - @Test - public void testScheduleBigDelay() { + public void testScheduleBigDelayNotOverflow() { EventLoopGroup group = new KQueueEventLoopGroup(1); final EventLoop el = group.next(); @@ -58,7 +37,7 @@ public void testScheduleBigDelay() { public void run() { // NOOP } - }, KQueueEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS); + }, Long.MAX_VALUE, TimeUnit.MILLISECONDS); assertFalse(future.awaitUninterruptibly(1000)); assertTrue(future.cancel(true)); diff --git a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java index b1e96e74973..d71e3f955dd 100644 --- a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java +++ b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java @@ -113,8 +113,6 @@ public Void run() { } } - static final long MAX_SCHEDULED_DAYS = 365 * 3; - /** * The NIO {@link Selector}. */ @@ -825,12 +823,4 @@ private void selectAgain() { logger.warn("Failed to update SelectionKeys.", t); } } - - @Override - protected void validateScheduled(long amount, TimeUnit unit) { - long days = unit.toDays(amount); - if (days > MAX_SCHEDULED_DAYS) { - throw new IllegalArgumentException("days: " + days + " (expected: < " + MAX_SCHEDULED_DAYS + ')'); - } - } } diff --git a/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java b/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java index 887dce421c9..124e0fe8a04 100644 --- a/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java +++ b/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java @@ -73,27 +73,8 @@ public void run() { } } - @Test(timeout = 5000L) - public void testScheduleBigDelayOverMax() { - EventLoopGroup group = new NioEventLoopGroup(1); - final EventLoop el = group.next(); - try { - el.schedule(new Runnable() { - @Override - public void run() { - // NOOP - } - }, Integer.MAX_VALUE, TimeUnit.DAYS); - fail(); - } catch (IllegalArgumentException expected) { - // expected - } - - group.shutdownGracefully(); - } - @Test - public void testScheduleBigDelay() { + public void testScheduleBigDelayNotOverflow() { EventLoopGroup group = new NioEventLoopGroup(1); final EventLoop el = group.next(); @@ -102,7 +83,7 @@ public void testScheduleBigDelay() { public void run() { // NOOP } - }, NioEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS); + }, Long.MAX_VALUE, TimeUnit.MILLISECONDS); assertFalse(future.awaitUninterruptibly(1000)); assertTrue(future.cancel(true));