Repository: netty/netty
Issue: OioSctpChannel read iterating over selected keys
Labels: defect

I'm curious why the [OioSctpChannel](https://github.com/netty/netty/blob/4.1/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java#L187) is iterating over `readSelector.selectedKeys()` but not using the `SelectionKey`.  It seems like the assumption is the number of keys that are active is an indication of how many read operation should be done?


Issue comments:
@benevans @trustin - Any thoughts?

/cc @jestan 

@jestan - Any objections to me removing this code?

@Scottmitch Please go ahead. We can't wait forever.

@trustin - Will do :)


Pull request review comments:

Patch that resolved the issue:
diff --git a/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java b/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java
index 6416ea4e60e..01a79d65ad5 100755
--- a/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java
+++ b/transport-sctp/src/main/java/io/netty/channel/sctp/oio/OioSctpChannel.java
@@ -184,39 +184,32 @@ protected int doReadMessages(List<Object> msgs) throws Exception {
             return readMessages;
         }
 
-        Set<SelectionKey> reableKeys = readSelector.selectedKeys();
-        try {
-            for (SelectionKey ignored : reableKeys) {
-                RecvByteBufAllocator.Handle allocHandle = this.allocHandle;
-                if (allocHandle == null) {
-                    this.allocHandle = allocHandle = config().getRecvByteBufAllocator().newHandle();
-                }
-                ByteBuf buffer = allocHandle.allocate(config().getAllocator());
-                boolean free = true;
+        RecvByteBufAllocator.Handle allocHandle = this.allocHandle;
+        if (allocHandle == null) {
+            this.allocHandle = allocHandle = config().getRecvByteBufAllocator().newHandle();
+        }
+        ByteBuf buffer = allocHandle.allocate(config().getAllocator());
+        boolean free = true;
 
-                try {
-                    ByteBuffer data = buffer.nioBuffer(buffer.writerIndex(), buffer.writableBytes());
-                    MessageInfo messageInfo = ch.receive(data, null, notificationHandler);
-                    if (messageInfo == null) {
-                        return readMessages;
-                    }
+        try {
+            ByteBuffer data = buffer.nioBuffer(buffer.writerIndex(), buffer.writableBytes());
+            MessageInfo messageInfo = ch.receive(data, null, notificationHandler);
+            if (messageInfo == null) {
+                return readMessages;
+            }
 
-                    data.flip();
-                    msgs.add(new SctpMessage(messageInfo, buffer.writerIndex(buffer.writerIndex() + data.remaining())));
-                    free = false;
-                    readMessages ++;
-                } catch (Throwable cause) {
-                    PlatformDependent.throwException(cause);
-                }  finally {
-                    int bytesRead = buffer.readableBytes();
-                    allocHandle.record(bytesRead);
-                    if (free) {
-                        buffer.release();
-                    }
-                }
+            data.flip();
+            msgs.add(new SctpMessage(messageInfo, buffer.writerIndex(buffer.writerIndex() + data.remaining())));
+            free = false;
+            readMessages ++;
+        } catch (Throwable cause) {
+            PlatformDependent.throwException(cause);
+        }  finally {
+            int bytesRead = buffer.readableBytes();
+            allocHandle.record(bytesRead);
+            if (free) {
+                buffer.release();
             }
-        } finally {
-            reableKeys.clear();
         }
         return readMessages;
     }


Test patch: