Regression in EPollArrayWrapper causes NPE when fd > 64 * 1024

Norman Maurer nmaurer at redhat.com
Mon Sep 16 20:02:02 UTC 2013


Hi Alan,

for sure I want to contribute a patch. Seems like the patches / reproducer are visible in the mail archive[1]. 

Anyway here are the files included inline:

Patch does:
* Eliminate the access to  the eventsHigh Map if "force" is true.
* Check for null before try to compare the stored events value.

# openjdk7 patch:

# HG changeset patch
# User Norman Maurer <nmaurer at redhat.com>
# Date 1379332618 -7200
# Node ID 4dbc07e1f92fa1c1b6c2dd20f20d67702aa91b6c
# Parent  861e489158effbf6a841119206eea2689fcb2a83
Fix NullPointerException which was thrown by EPollArrayWrapper.setUpdateEvents(...) if fd > 64 * 1024 was used.

diff -r 861e489158ef -r 4dbc07e1f92f src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
--- a/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java	Thu Sep 12 17:17:40 2013 -0700
+++ b/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java	Mon Sep 16 13:56:58 2013 +0200
@@ -175,12 +175,18 @@
             }
         } else {
             Integer key = Integer.valueOf(fd);
-            if ((eventsHigh.get(key) != KILLED) || force) {
-                eventsHigh.put(key, Byte.valueOf(events));
+            if (force || isEventsHighNotKilled(key)) {
+                Byte value = Byte.valueOf(events);
+                eventsHigh.put(key, value);
             }
         }
     }
 
+    private boolean isEventsHighNotKilled(Integer key) {
+        Byte value = eventsHigh.get(key);
+        return value == null || value != KILLED;
+    }
+
     /**
      * Returns the pending update events for the given file descriptor.
      */


# openjdk8 patch:

# HG changeset patch
# User Norman Maurer <nmaurer at redhat.com>
# Date 1379332812 -7200
# Node ID 5023e9319a3815c52654232b21258f4670732c8e
# Parent  b67c8099ba28b66ccafa85abc096ac592089ad00
Fix NullPointerException which was thrown by EPollArrayWrapper.setUpdateEvents(...) if fd > 64 * 1024 was used.

diff -r b67c8099ba28 -r 5023e9319a38 src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
--- a/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java	Thu Sep 12 11:09:11 2013 -0700
+++ b/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java	Mon Sep 16 14:00:12 2013 +0200
@@ -175,12 +175,18 @@
             }
         } else {
             Integer key = Integer.valueOf(fd);
-            if ((eventsHigh.get(key) != KILLED) || force) {
-                eventsHigh.put(key, Byte.valueOf(events));
+            if (force || isEventsHighNotKilled(key)) {
+                Byte value = Byte.valueOf(events);
+                eventsHigh.put(key, value);
             }
         }
     }
 
+    private boolean isEventsHighNotKilled(Integer key) {
+        Byte value = eventsHigh.get(key);
+        return value == null || value != KILLED;
+    }
+
     /**
      * Returns the pending update events for the given file descriptor.
      */


# BugReproducer.java:

import java.net.InetSocketAddress;
import java.net.StandardSocketOptions;
import java.nio.channels.SelectionKey;
import java.nio.channels.Selector;
import java.nio.channels.ServerSocketChannel;
import java.nio.channels.SocketChannel;
import java.util.Iterator;

/**
 * @author <a href="mailto:nmaurer at redhat.com">Norman Maurer</a>
 */
public class BugReproducer {

    public static void main(String args[]) throws Exception {
        int connections = 0;
        Selector selector = Selector.open();

        final ServerSocketChannel channel = ServerSocketChannel.open();
        channel.configureBlocking(false);
        channel.register(selector, SelectionKey.OP_ACCEPT, channel);
        channel.bind(new InetSocketAddress("0.0.0.0", Integer.parseInt(args[0])));

        for (;;) {
            selector.select();
            Iterator<SelectionKey> keys = selector.selectedKeys().iterator();

            while (keys.hasNext()) {
                SelectionKey key = keys.next();
                keys.remove();
                if (key.isAcceptable()) {
                    ServerSocketChannel ch = (ServerSocketChannel) key.attachment();
                    for (;;) {
                        SocketChannel sch = ch.accept();
                        if (sch == null) {
                            break;
                        }
                        sch.setOption(StandardSocketOptions.SO_KEEPALIVE, true);
                        sch.setOption(StandardSocketOptions.SO_REUSEADDR, true);
                        sch.configureBlocking(false);
                        sch.register(selector, SelectionKey.OP_READ);

                        System.out.println(++connections);
                    }
                }

            }
        }
    }
}

# BugReproducerClient.java:

import java.net.InetSocketAddress;
import java.net.StandardSocketOptions;
import java.nio.channels.SelectionKey;
import java.nio.channels.Selector;
import java.nio.channels.SocketChannel;
import java.util.Iterator;

/**
 * @author <a href="mailto:nmaurer at redhat.com">Norman Maurer</a>
 */
public class BugReproducerClient {

    private static final int MAX_CONNECTIONS = 40 * 1024;
    public static void main(String args[]) throws Exception {

        InetSocketAddress address = new InetSocketAddress(args[0], Integer.parseInt(args[1]));
        int connections = 0;
        Selector selector = Selector.open();

        for (;;) {
            SocketChannel channel = SocketChannel.open();
            channel.configureBlocking(false);
            channel.setOption(StandardSocketOptions.SO_KEEPALIVE, true);
            channel.setOption(StandardSocketOptions.SO_REUSEADDR, true);
            if (!channel.connect(address)) {
                channel.register(selector, SelectionKey.OP_CONNECT);

                selector.select();
                Iterator<SelectionKey> keys = selector.selectedKeys().iterator();

                while (keys.hasNext()) {
                    SelectionKey key = keys.next();
                    keys.remove();
                    if (key.isConnectable()) {
                        key.interestOps((key.interestOps() &~SelectionKey.OP_CONNECT) | SelectionKey.OP_READ);
                        System.out.println(++connections);
                    }

                }
            } else {
                channel.register(selector, SelectionKey.OP_READ);
                System.out.println(++connections);
            }
            if (MAX_CONNECTIONS == connections) {
                Thread.sleep(1000 * 60);
                break;
            } else {
                if (connections % 500 == 0) {
                    Thread.sleep(100);
                }
            }

        }
    }
}

Let me know if you need anything else.

[1 http://mail.openjdk.java.net/pipermail/nio-dev/2013-September/002284.html] 
---
Norman Maurer
nmaurer at redhat.com

JBoss, by Red Hat



Am 16.09.2013 um 19:49 schrieb Alan Bateman <Alan.Bateman at oracle.com>:

> On 16/09/2013 16:45, Norman Maurer wrote:
>> 
>> Hi there,
>> 
>> this is my first bug-report here so bear with me if something is missing ;)
>> 
>> During testing netty.io with many concurrent connections one of our users reported a NullPointerException which was thrown by sun.nio.ch.EPollArrayWrapper.setUpdateEvents(...). This was observed as soon as the concurrent connection count > 64 * 1024.
> Thanks for this (and the analysis), embarrassing as this has been in 7u40 for a long time and doesn't seem to be have been noticed. Maybe there aren't too many people testing with this number of connections.
> 
> I've created this bug to track it:
>   8024883: (se) SelectableChannel.register throws NPE of fd >= 64k (lnx)
> 
> and just did a few initial testing with the attached. Your mail mentions attachments but they seem to have been dropped. If you are looking to contribute a patch then you can re-send with the patch inlined?
> 
> -Alan
> 
> 
> diff --git a/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java b/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
> --- a/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
> +++ b/src/solaris/classes/sun/nio/ch/EPollArrayWrapper.java
> @@ -175,7 +175,8 @@
>              }
>          } else {
>              Integer key = Integer.valueOf(fd);
> -            if ((eventsHigh.get(key) != KILLED) || force) {
> +            Byte prev = eventsHigh.get(key);
> +            if (prev == null || prev == KILLED || force) {
>                  eventsHigh.put(key, Byte.valueOf(events));
>              }
>          }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20130916/36a6b8e1/attachment-0001.html 


More information about the nio-dev mailing list