Two issues with Socket.wait()

[dylanneild]

Hi there,

I've been doing some work on the Kitura project and in sketching out some ideas for performance optimizations I've found what I think are a couple of issues / improvements to be made to Socket.wait():

First:

In the wait() function a time is initialized:

var timer = timeval()

Some code following this computes a time based on the timeout UInt that gets passed to wait() in the event that timeout is greater than 0.

If, however, you pass 0 as a timeout (to NOT use a timeout at all and just select() forever), the result is that select() instantly returns as the timer fires immediately.

So I would suggest modifying this line:

let count = select(highSocketfd + 1, &readfds, nil, nil, &timer)

With some code that passes nil in place of &timer in the event that a 0 timeout is passed to the wait() function.

Secondly:

At the start of wait() the sockets in the socket group are looped and any socket with an invalid descriptor causes an exception to throw. All good.

The problem is that each socket is also checked for "isConnected" and any sockets where isConnected = false also throw an exception.

This basically means you can't use wait() to monitor sockets that are both signalling that data is ready to be read AS WELL as listen() sockets waiting for connections (which are technically not connected).

So, I'd suggest changing this one piece of code:

if !socket.isConnected {                
   throw Error(code: Socket.SOCKET_ERR_NOT_CONNECTED, reason: nil)
}

To:

if !socket.isConnected && !socket.isListening {             
   throw Error(code: Socket.SOCKET_ERR_NOT_CONNECTED, reason: nil)
}

This would allow listen() sockets to be bundled in to the select(). Most helpful!

I'd open a PR but I'm knee deep in toolchain 2016-06-05 right now and I see BlueSocket is already tagged ahead to a 2016-06-20 dependency.

[billabt]

No problem. I'm on vacation this week but will try to find some time to take a look. Thanks.

[billabt]

In order to not lose existing functionality to allow a quick check (i.e. check and return immediately), I've modified the API so that we can do both, the quick check plus the indefinite wait.

    ///
    /// Monitor an array of sockets, returning when data is available or timeout occurs.
    ///
    /// - Parameters:
    ///     - sockets:      An array of sockets to be monitored.
    ///     - timeout:      Timeout (in msec) before returning.  A timeout value of 0 will return immediately.
    ///     - waitForever:  If true, this function will wait indefinitely regardless of timeout value. Defaults to false.
    ///
    /// - Returns: An optional array of sockets which have data available or nil if a timeout expires.
    ///
    public class func wait(for sockets: [Socket], timeout: UInt, waitForever: Bool = false) throws -> [Socket]?

[dylanneild]

Looks great, thanks! -d