Proper handling of epoll_ctl errors

Motivation:

When using epoll_ctl we should respect the return value and do the right thing depending on it.

Modifications:

Adjust java and native code to respect epoll_ctl return values.

Result:

Correct and cleaner code.

transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.c

@@ -686,27 +686,30 @@ JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_epollWait0(JNIEnv* env
     return ready;
 }
 
-JNIEXPORT void JNICALL Java_io_netty_channel_epoll_Native_epollCtlAdd(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags) {
+JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_epollCtlAdd0(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags) {
-    if (epollCtl(env, efd, EPOLL_CTL_ADD, fd, flags) < 0) {
+    int res = epollCtl(env, efd, EPOLL_CTL_ADD, fd, flags);
-        int err = errno;
+    if (res < 0) {
-        throwRuntimeException(env, exceptionMessage("epoll_ctl() failed: ", err));
+        return -errno;
     }
+    return res;
 }
 
-JNIEXPORT void JNICALL Java_io_netty_channel_epoll_Native_epollCtlMod(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags) {
+JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_epollCtlMod0(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags) {
-    if (epollCtl(env, efd, EPOLL_CTL_MOD, fd, flags) < 0) {
+    int res = epollCtl(env, efd, EPOLL_CTL_MOD, fd, flags);
-        int err = errno;
+    if (res < 0) {
-        throwRuntimeException(env, exceptionMessage("epoll_ctl() failed: ", err));
+        return -errno;
     }
+    return res;
 }
 
-JNIEXPORT void JNICALL Java_io_netty_channel_epoll_Native_epollCtlDel(JNIEnv* env, jclass clazz, jint efd, jint fd) {
+JNIEXPORT jint JNICALL Java_io_netty_channel_epoll_Native_epollCtlDel0(JNIEnv* env, jclass clazz, jint efd, jint fd) {
     // Create an empty event to workaround a bug in older kernels which can not handle NULL.
     struct epoll_event event = { 0 };
-    if (epoll_ctl(efd, EPOLL_CTL_DEL, fd, &event) < 0) {
+    int res = epoll_ctl(efd, EPOLL_CTL_DEL, fd, &event);
-        int err = errno;
+    if (res < 0) {
-        throwRuntimeException(env, exceptionMessage("epoll_ctl() failed: ", err));
+        return -errno;
     }
+    return res;
 }
 
 static inline jint _write(JNIEnv* env, jclass clazz, jint fd, void* buffer, jint pos, jint limit) {

transport-native-epoll/src/main/c/io_netty_channel_epoll_Native.h

@@ -38,9 +38,9 @@ void Java_io_netty_channel_epoll_Native_eventFdWrite(JNIEnv* env, jclass clazz,
 void Java_io_netty_channel_epoll_Native_eventFdRead(JNIEnv* env, jclass clazz, jint fd);
 jint Java_io_netty_channel_epoll_Native_epollCreate(JNIEnv* env, jclass clazz);
 jint Java_io_netty_channel_epoll_Native_epollWait0(JNIEnv* env, jclass clazz, jint efd, jlong address, jint length, jint timeout);
-void Java_io_netty_channel_epoll_Native_epollCtlAdd(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags);
+jint Java_io_netty_channel_epoll_Native_epollCtlAdd0(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags);
-void Java_io_netty_channel_epoll_Native_epollCtlMod(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags);
+jint Java_io_netty_channel_epoll_Native_epollCtlMod0(JNIEnv* env, jclass clazz, jint efd, jint fd, jint flags);
-void  Java_io_netty_channel_epoll_Native_epollCtlDel(JNIEnv* env, jclass clazz, jint efd, jint fd);
+jint Java_io_netty_channel_epoll_Native_epollCtlDel0(JNIEnv* env, jclass clazz, jint efd, jint fd);
 jint Java_io_netty_channel_epoll_Native_write0(JNIEnv* env, jclass clazz, jint fd, jobject jbuffer, jint pos, jint limit);
 jint Java_io_netty_channel_epoll_Native_writeAddress0(JNIEnv* env, jclass clazz, jint fd, jlong address, jint pos, jint limit);
 jlong Java_io_netty_channel_epoll_Native_writev0(JNIEnv* env, jclass clazz, jint fd, jobjectArray buffers, jint offset, jint length);

transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollChannel.java

@@ -28,6 +28,7 @@ import io.netty.channel.unix.UnixChannel;
 import io.netty.util.ReferenceCountUtil;
 import io.netty.util.internal.OneTimeTask;
 
+import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.nio.channels.UnresolvedAddressException;
@@ -59,14 +60,14 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann
         fileDescriptor = fd;
     }
 
-    void setFlag(int flag) {
+    void setFlag(int flag) throws IOException {
         if (!isFlagSet(flag)) {
             flags |= flag;
             modifyEvents();
         }
     }
 
-    void clearFlag(int flag) {
+    void clearFlag(int flag) throws IOException {
         if (isFlagSet(flag)) {
             flags &= ~flag;
             modifyEvents();
@@ -160,7 +161,7 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann
         }
     }
 
-    private void modifyEvents() {
+    private void modifyEvents() throws IOException {
         if (isOpen() && isRegistered()) {
             ((EpollEventLoop) eventLoop()).modify(this);
         }
@@ -325,7 +326,15 @@ abstract class AbstractEpollChannel extends AbstractChannel implements UnixChann
         }
 
         protected final void clearEpollIn0() {
-            clearFlag(readFlag);
+            assert eventLoop().inEventLoop();
+            try {
+                clearFlag(readFlag);
+            } catch (IOException e) {
+                // When this happens there is something completely wrong with either the filedescriptor or epoll,
+                // so fire the exception through the pipeline and close the Channel.
+                pipeline().fireExceptionCaught(e);
+                unsafe().close(unsafe().voidPromise());
+            }
         }
     }
 }

transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelConfig.java

@@ -16,11 +16,13 @@
 package io.netty.channel.epoll;
 
 import io.netty.buffer.ByteBufAllocator;
+import io.netty.channel.ChannelException;
 import io.netty.channel.ChannelOption;
 import io.netty.channel.DefaultChannelConfig;
 import io.netty.channel.MessageSizeEstimator;
 import io.netty.channel.RecvByteBufAllocator;
 
+import java.io.IOException;
 import java.util.Map;
 
 public class EpollChannelConfig extends DefaultChannelConfig {
@@ -133,7 +135,8 @@ public class EpollChannelConfig extends DefaultChannelConfig {
         if (mode == null) {
             throw new NullPointerException("mode");
         }
-        switch (mode) {
+        try {
+            switch (mode) {
             case EDGE_TRIGGERED:
                 checkChannelNotRegistered();
                 channel.setFlag(Native.EPOLLET);
@@ -143,7 +146,10 @@ public class EpollChannelConfig extends DefaultChannelConfig {
                 channel.clearFlag(Native.EPOLLET);
                 break;
             default:
-                throw  new Error();
+                throw new Error();
+            }
+        } catch (IOException e) {
+            throw new ChannelException(e);
         }
         return this;
     }

transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java

@@ -73,7 +73,11 @@ final class EpollEventLoop extends SingleThreadEventLoop {
         try {
             this.epollFd = epollFd = Native.epollCreate();
             this.eventFd = eventFd = Native.eventFd();
-            Native.epollCtlAdd(epollFd, eventFd, Native.EPOLLIN);
+            try {
+                Native.epollCtlAdd(epollFd, eventFd, Native.EPOLLIN);
+            } catch (IOException e) {
+                throw new IllegalStateException("Unable to add eventFd filedescriptor to epoll", e);
+            }
             success = true;
         } finally {
             if (!success) {
@@ -106,7 +110,7 @@ final class EpollEventLoop extends SingleThreadEventLoop {
     /**
      * Register the given epoll with this {@link io.netty.channel.EventLoop}.
      */
-    void add(AbstractEpollChannel ch) {
+    void add(AbstractEpollChannel ch) throws IOException {
         assert inEventLoop();
         int fd = ch.fd().intValue();
         Native.epollCtlAdd(epollFd, fd, ch.flags);
@@ -116,15 +120,15 @@ final class EpollEventLoop extends SingleThreadEventLoop {
     /**
      * The flags of the given epoll was modified so update the registration
      */
-    void modify(AbstractEpollChannel ch) {
+    void modify(AbstractEpollChannel ch) throws IOException {
         assert inEventLoop();
         Native.epollCtlMod(epollFd, ch.fd().intValue(), ch.flags);
     }
 
     /**
-     * Deregister the given epoll from this {@link io.netty.channel.EventLoop}.
+     * Deregister the given epoll from this {@link EventLoop}.
      */
-    void remove(AbstractEpollChannel ch) {
+    void remove(AbstractEpollChannel ch) throws IOException {
         assert inEventLoop();
 
         if (ch.isOpen()) {
@@ -326,7 +330,14 @@ final class EpollEventLoop extends SingleThreadEventLoop {
                     }
                 } else {
                     // We received an event for an fd which we not use anymore. Remove it from the epoll_event set.
-                    Native.epollCtlDel(epollFd, fd);
+                    try {
+                        Native.epollCtlDel(epollFd, fd);
+                    } catch (IOException ignore) {
+                        // This can happen but is nothing we need to worry about as we only try to delete
+                        // the fd from the epoll set as we not found it in our mappings. So this call to
+                        // epollCtlDel(...) is just to ensure we cleanup stuff and so may fail if it was
+                        // deleted before or the file descriptor was closed before.
+                    }
                 }
             }
         }

transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java

@@ -156,10 +156,29 @@ public final class Native {
     }
     private static native int epollWait0(int efd, long address, int len, int timeout);
 
-    public static native void epollCtlAdd(int efd, final int fd, final int flags);
+    public static void epollCtlAdd(int efd, final int fd, final int flags) throws IOException {
+        int res = epollCtlAdd0(efd, fd, flags);
+        if (res < 0) {
+            throw newIOException("epoll_ctl", res);
+        }
+    }
+    private static native int epollCtlAdd0(int efd, final int fd, final int flags);
 
-    public static native void epollCtlMod(int efd, final int fd, final int flags);
+    public static void epollCtlMod(int efd, final int fd, final int flags) throws IOException {
-    public static native void epollCtlDel(int efd, final int fd);
+        int res = epollCtlMod0(efd, fd, flags);
+        if (res < 0) {
+            throw newIOException("epoll_ctl", res);
+        }
+    }
+    private static native int epollCtlMod0(int efd, final int fd, final int flags);
+
+    public static void epollCtlDel(int efd, final int fd) throws IOException {
+        int res = epollCtlDel0(efd, fd);
+        if (res < 0) {
+            throw newIOException("epoll_ctl", res);
+        }
+    }
+    private static native int epollCtlDel0(int efd, final int fd);
 
     private static native int errnoEBADF();
     private static native int errnoEPIPE();