ArrayIndexOutOfBoundsException in IcmpNative.FD_ISSET

[Brikman]

Got the following exception:

Exception in thread "icmp-pinger-7" java.lang.ArrayIndexOutOfBoundsException: Index 321 out of bounds for length 128
    at com.zaxxer.ping.impl.IcmpNativeKt.FD_ISSET(IcmpNative.kt:231)

I have highly loaded application, which send a lot of pings to network devices (from hundreds to thousands requests per second), and I've caught this error several times, but honestly can't determine the conditions to reproduce it artifically.

I've been researching the code of IcmpPinger for some time (actually it's not simple, some kind of a C-code port into kotlin world). It seems like fd4 becomes too big at some point of time, which caused the ndx to become out of bounds for 128-length array (in my case, ndx=321, fd4=321*64=20544):

fun FD_ISSET(fd:Int, fd_set:Fd_set) : Boolean {
   val ndx = fd / 64 // <============================== ndx = 321
   val currvalue = fd_set.fds_bits[ndx].get() <======== ArrayIndexOutOfBoundsException
   val andValue = (1L shl (fd % 64))

   return currvalue and andValue != 0L
}

---

Does anyone have an idea of how the descriptor fd4 became too big and is it a bug in library, or some rare case under high load that this pinger is not designed for?

[Brikman]

Got it.

Due to Fd_set uses the array fds_bits of size 128, the maximum value that sockets fd4 and fd6 could have is 128 * 64 = 8192.

In other words, the whole implementation of IcmpPinger assumes that there is no more than 8190 file descriptors opened in the OS at the moment when IcmpPinger initializes their sockets.

So, I think, this is a serious drawback or at least a fat remark of using this library, because it's pretty easy to rich that limit and get your pinger instance dead without chance to fix it. When main loop in runSelector() fails with exception, resources (sockets fd4 and fd6) did not closed event if you call stopSelector(), and you get an unfixable FD leak.

---

If the Fd_set implementation is hardly depends on array size of 128, I think it makes sense to throw an exception in createSockets() if fd received from libc.socket(...) is greater than 8192. It's better to fail-fast with meaningfull and explainable error than with ArrayIndexOutOfBoundsException which breaks the main loop.

---

@brettwooldridge, I woold be greatful if you judge me. You library is great and the only one that suggests such functionality in the whole java world, and we're going to use it anyway because our project now relies heavily on it because have no other choice.

[Brikman]

The simpliest way to reproduce the error:

private fun getOpenFileDescriptorCount(): Long =
    (ManagementFactory.getOperatingSystemMXBean() as UnixOperatingSystemMXBean).openFileDescriptorCount

fun main() {
    println("Opened FD: ${getOpenFileDescriptorCount()}")

    Array(9000) { ServerSocket(0) }

    println("Opened FD: ${getOpenFileDescriptorCount()}")

    val pinger = IcmpPinger(Handler)
    val pingerThread = Thread(null, { pinger.runSelector() }, "pinger-thread").apply { start() }

    pinger.ping(PingTarget(InetAddress.getByName("10.0.0.1")))

    pingerThread.join()
}

object Handler : PingResponseHandler {
    override fun onResponse(pingTarget: PingTarget, responseTimeSec: Double, byteCount: Int, seq: Int) {
        println("PING ${pingTarget.inetAddress}: SUCCESS")
    }

    override fun onTimeout(pingTarget: PingTarget) {
        println("PING ${pingTarget.inetAddress}: FAILURE")
    }
}

[brettwooldridge]

Thank you for the investigation. Is it possible for you to propose a Pull Request with either a fix to remove the limit, or improved error handling?