Repository: netty/netty
Issue: EventLoop.schedule with big delay fails
Labels: defect

this behaviour was discovered by accident, so there is no actual usecase behind it but it is definitely not possible to use Long.MAX_VALUE in milliseconds for a scheduled event

### Expected behavior
runnable should trigger after given delay
getting a proper exception if something goes wrong

### Actual behavior
working with delays smaller of around 10_000_000_000ms on all platforms i tested with (osx, linux)
failing on osx with already a delay of 15_768_000_000ms (5 years)
with the following exception
```
Nov 04, 2017 2:08:39 PM io.netty.channel.nio.NioEventLoop handleLoopException
WARNING: Unexpected exception in the selector loop.
java.io.IOException: Invalid argument
	at sun.nio.ch.KQueueArrayWrapper.kevent0(Native Method)
	at sun.nio.ch.KQueueArrayWrapper.poll(KQueueArrayWrapper.java:198)
	at sun.nio.ch.KQueueSelectorImpl.doSelect(KQueueSelectorImpl.java:117)
	at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
	at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
	at io.netty.channel.nio.NioEventLoop.select(NioEventLoop.java:746)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:391)
	at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
	at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:144)
	at java.lang.Thread.run(Thread.java:748)
```
failing on linux with around 946_080_000_000 (300 years)
which immediately triggers the runnable without exception

### Minimal yet complete reproducer code (or URL to code)
```
public class Main {
    public static void main(String[] args) {
        final ThreadFactory threadFactory = new DefaultThreadFactory("test-eventloop");
        NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(1, threadFactory);
        eventLoopGroup.setIoRatio(50);
        EventLoop el = eventLoopGroup.next();
        Runnable toRun = Main::handle;
        Future<?> future = el.schedule(toRun, 1000L * 60 * 60 * 24 * 365 * 300, TimeUnit.MILLISECONDS);
        System.out.println("timer scheduled");
    }

    private static void handle() {
        System.out.println("timer fired");
    }
}
```

### Netty version
seems to happen on all 4.1.x versions (tested on 4.1.8, 4.1.15, 4.1.16)

### JVM version (e.g. `java -version`)
osx oracle 1.8.0_152
linux openjdk 1.8.0_144

### OS version (e.g. `uname -a`)
darwin 16.7.0
linux 4.13.9 and 4.13.11

Issue comments:
will have a look. Most likely its an overflow.
@zyclonite please check https://github.com/netty/netty/pull/7402
thank you for the fast fix!
works for linux so far (will test it for osx in the evening)

typo or on purpose? ```static final long MAX_SCHEDULED_DAYS = 356 * 3;```
works on osx as well and shows consistent behaviour
fixed by https://github.com/netty/netty/pull/7402

Pull request review comments:
please note that I used this as it seems to work on linux / Windows and macOS.
Why do we care about windows and linux here? This is in the kqueue transport.
add a comment explaining where this number comes from.
https://www.freebsd.org/cgi/man.cgi?query=kqueue&sektion=2#end

>  The timeout value is limited to 24	hours; longer timeouts will be
     silently reinterpreted as 24 hours.

Do we need to do anything for kqueue? IIRC we verified that scheduled tasks are actually expired even if we are woken up as a result of timeout.
Is this limit defined in the docs, or is this just based upon the size of the data types? If its data type based this may not be correct and depends upon the type widths. For example `long` may be unsigned 32, but IIRC it is only guaranteed to be smaller than `int`. `time_t` maybe 32 or 64 byte signed integer. If this is based upon datatypes we should get this information via JNI ... hopefully in a limits file somewhere?

make this a java doc comment?
its defined in the manpage to be the max you can set.
will do... It came from testing ;)
> EINVAL new_value is not properly initialized (one of the tv_nsec
              falls outside the range zero to 999,999,999).

Gotcha ... consider adding to javadocs.
`TimeUnit.NANOSECONDS.toSeconds(999999999)`

That evaluates to zero.  Is this calculation correct?
we can just remove the `TimeUnit.NANOSECONDS.toSeconds(999999999)` here and keep it simple. Will do
@Scottmitch sorry this was confusing. I tested the value with NIO on linux, windows and macOs ;)
@Scottmitch oh my ... This is a nice "bug". I wonder if this is still true and why they not just fixed it... any idea ?
So if the variable is named `MAX_SCHEDULED_DAYS`   why are there no "day" like constants in the arithmetic?  It's kind of odd to put a "seconds" value into a "days" variable.
carl because I am stupid... I forgot to push the latest change :(
s/356/365/ ?

Patch that resolved the issue:
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 3043b1709fe..795aeba907c 100644
--- a/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java
+++ b/common/src/main/java/io/netty/util/concurrent/AbstractScheduledEventExecutor.java
@@ -150,6 +150,8 @@ public ScheduledFuture<?> schedule(Runnable command, long delay, TimeUnit unit)
         if (delay < 0) {
             delay = 0;
         }
+        validateScheduled(delay, unit);
+
         return schedule(new ScheduledFutureTask<Void>(
                 this, command, null, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay))));
     }
@@ -161,6 +163,8 @@ public <V> ScheduledFuture<V> schedule(Callable<V> callable, long delay, TimeUni
         if (delay < 0) {
             delay = 0;
         }
+        validateScheduled(delay, unit);
+
         return schedule(new ScheduledFutureTask<V>(
                 this, callable, ScheduledFutureTask.deadlineNanos(unit.toNanos(delay))));
     }
@@ -177,6 +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);
 
         return schedule(new ScheduledFutureTask<Void>(
                 this, Executors.<Void>callable(command, null),
@@ -196,11 +202,21 @@ public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialD
                     String.format("delay: %d (expected: > 0)", delay));
         }
 
+        validateScheduled(initialDelay, unit);
+        validateScheduled(delay, unit);
+
         return schedule(new ScheduledFutureTask<Void>(
                 this, Executors.<Void>callable(command, null),
                 ScheduledFutureTask.deadlineNanos(unit.toNanos(initialDelay)), -unit.toNanos(delay)));
     }
 
+    /**
+     * Sub-classes may override this to restrict the maximal amount of time someone can use to schedule a task.
+     */
+    protected void validateScheduled(long amount, TimeUnit unit) {
+        // NOOP
+    }
+
     <V> ScheduledFuture<V> schedule(final ScheduledFutureTask<V> task) {
         if (inEventLoop()) {
             scheduledTaskQueue().add(task);
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 b7d02283e79..cf55ef67ec5 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,6 +37,7 @@
 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;
@@ -78,6 +79,9 @@ public Integer call() throws Exception {
     private volatile int wakenUp;
     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);
+
     EpollEventLoop(EventLoopGroup parent, Executor executor, int maxEvents,
                    SelectStrategy strategy, RejectedExecutionHandler rejectedExecutionHandler) {
         super(parent, executor, false, DEFAULT_MAX_PENDING_TASKS, rejectedExecutionHandler);
@@ -449,4 +453,12 @@ 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-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java b/transport-native-kqueue/src/main/java/io/netty/channel/kqueue/KQueueEventLoop.java
index 5af59ba912c..3badcea96c7 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,6 +33,7 @@
 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;
@@ -76,6 +77,8 @@ 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);
@@ -367,4 +370,12 @@ 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/src/main/java/io/netty/channel/nio/NioEventLoop.java b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
index c4434cf448b..b1e96e74973 100644
--- a/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
+++ b/transport/src/main/java/io/netty/channel/nio/NioEventLoop.java
@@ -113,6 +113,8 @@ public Void run() {
         }
     }
 
+    static final long MAX_SCHEDULED_DAYS = 365 * 3;
+
     /**
      * The NIO {@link Selector}.
      */
@@ -730,6 +732,7 @@ private void select(boolean oldWakenUp) throws IOException {
             int selectCnt = 0;
             long currentTimeNanos = System.nanoTime();
             long selectDeadLineNanos = currentTimeNanos + delayNanos(currentTimeNanos);
+
             for (;;) {
                 long timeoutMillis = (selectDeadLineNanos - currentTimeNanos + 500000L) / 1000000L;
                 if (timeoutMillis <= 0) {
@@ -822,4 +825,12 @@ 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 + ')');
+        }
+    }
 }


Test patch:
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
new file mode 100644
index 00000000000..0fe824b334d
--- /dev/null
+++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollEventLoopTest.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2017 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.channel.epoll;
+
+import io.netty.channel.EventLoop;
+import io.netty.channel.EventLoopGroup;
+import io.netty.util.concurrent.Future;
+import org.junit.Test;
+
+import java.util.concurrent.TimeUnit;
+
+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() {
+        EventLoopGroup group = new EpollEventLoopGroup(1);
+
+        final EventLoop el = group.next();
+        Future<?> future = el.schedule(new Runnable() {
+            @Override
+            public void run() {
+                // NOOP
+            }
+        }, EpollEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS);
+
+        assertFalse(future.awaitUninterruptibly(1000));
+        assertTrue(future.cancel(true));
+        group.shutdownGracefully();
+    }
+}
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
new file mode 100644
index 00000000000..c7ad56e37ab
--- /dev/null
+++ b/transport-native-kqueue/src/test/java/io/netty/channel/kqueue/KQueueEventLoopTest.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2017 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.channel.kqueue;
+
+import io.netty.channel.EventLoop;
+import io.netty.channel.EventLoopGroup;
+import io.netty.util.concurrent.Future;
+import org.junit.Test;
+
+import java.util.concurrent.TimeUnit;
+
+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() {
+        EventLoopGroup group = new KQueueEventLoopGroup(1);
+
+        final EventLoop el = group.next();
+        Future<?> future = el.schedule(new Runnable() {
+            @Override
+            public void run() {
+                // NOOP
+            }
+        }, KQueueEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS);
+
+        assertFalse(future.awaitUninterruptibly(1000));
+        assertTrue(future.cancel(true));
+        group.shutdownGracefully();
+    }
+}
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 583ad858eee..aad29b72036 100644
--- a/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java
+++ b/transport/src/test/java/io/netty/channel/nio/NioEventLoopTest.java
@@ -17,12 +17,15 @@
 
 import io.netty.channel.AbstractEventLoopTest;
 import io.netty.channel.Channel;
+import io.netty.channel.EventLoop;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.socket.ServerSocketChannel;
 import io.netty.channel.socket.nio.NioServerSocketChannel;
+import io.netty.util.concurrent.Future;
 import org.junit.Test;
 
 import java.nio.channels.Selector;
+import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.*;
 
@@ -68,4 +71,40 @@ public void run() {
             group.shutdownGracefully();
         }
     }
+
+    @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() {
+        EventLoopGroup group = new NioEventLoopGroup(1);
+
+        final EventLoop el = group.next();
+        Future<?> future = el.schedule(new Runnable() {
+            @Override
+            public void run() {
+                // NOOP
+            }
+        }, NioEventLoop.MAX_SCHEDULED_DAYS, TimeUnit.DAYS);
+
+        assertFalse(future.awaitUninterruptibly(1000));
+        assertTrue(future.cancel(true));
+        group.shutdownGracefully();
+    }
 }