Timeout setting never has an effect in tcp_client.lua

[lharder]

Hi Britzl,

I have just stumbled over what I think must be an unintended behaviour in your tcp_client.

You set the timeout parameter for tcp connections to 0 statically, so the client may block indefinitely. While that may be intended(?), even if you decided to set the timeout to a value > 0, it would never have an effect as you first try to connect and only then set the timeout:

63:   client_socket = socket.tcp()
64:   assert(client_socket:connect(server_ip, server_port))
65:   assert(client_socket:settimeout(0))

I would suggest to both allow for a user to optionally define a timeout parameter when creating a new tcp_client and swap lines 64 and 65, so that it would have the desired effect. The whole snippet would then be:

function M.create(server_ip, server_port, on_data, on_disconnect, timeout)
    assert(server_ip, "You must provide a server_ip")
    assert(server_port, "You must provide a server_port")
    assert(on_data, "You must provide an on_data callback function")
    assert(on_disconnect, "You must provide an on_disconnect callback function")
        if timeout == nil then timeout = 0 end

    log("Creating TCP client", server_ip, server_port)

    local client = {
        pattern = "*l",
    }

    local client_socket = nil
    local send_queue = nil
    local client_socket_table = nil

    local ok, err = pcall(function()
        client_socket = socket.tcp()
        assert(client_socket:settimeout( timeout ))
        assert(client_socket:connect(server_ip, server_port))
        client_socket_table = { client_socket }
        send_queue = tcp_send_queue.create(client_socket, M.TCP_SEND_CHUNK_SIZE)
    end)

I am attaching a file with the intended change that I have tested and which works fine for me. It is a non-breaking change as far as I can see and I would appreciate it if you accepted it in your main branch.

Cheers, Lutz

tcp_client.lua.zip

[britzl]

The purpose of the client_socket:settimeout(0) after connect() is so that any future send, receive, and accept operation on the socket will not block at all.

The documentation states that:

"By default, all I/O operations are blocking. That is, any call to the methods send, receive, and accept will block indefinitely, until the operation completes. The settimeout method defines a limit on the amount of time the I/O methods can block. When a timeout is set and the specified amount of time has elapsed, the affected methods give up and fail with an error code."

If you wish for operations to block then setting "the nil timeout value allows operations to block indefinitely." Remember that in Lua 0 is not considered false/nil as in for instance C.

https://lunarmodules.github.io/luasocket/tcp.html#settimeout

[lharder]

Hi Britzl, ah, ok, if that behaviour is intended, never mind.

Strange, when I try to connect to an ip address that does not respond, my program blocks and never timeouts. With the change I suggested, that case works for me. But maybe there is a problem in my code which I misinterpreted. Thanks for your feedback.

[britzl]

Strange, when I try to connect to an ip address that does not respond, my program blocks and never timeouts.

Which OS are you testing on?

With the change I suggested, that case works for me

Ok, hmm, perhaps we could change function signature to create(server_ip, server_port, on_data, on_disconnect, options) with options being a table with configuration options. One could be connect_timeout which is specifically used for the call to client_socket:connect(server_ip, server_port). Something like this:

function M.create(server_ip, server_port, on_data, on_disconnect, options)
    assert(server_ip, "You must provide a server_ip")
    assert(server_port, "You must provide a server_port")
    assert(on_data, "You must provide an on_data callback function")
    assert(on_disconnect, "You must provide an on_disconnect callback function")

    log("Creating TCP client", server_ip, server_port)

    local client = {
        pattern = "*l",
    }

    local client_socket = nil
    local send_queue = nil
    local client_socket_table = nil
    local connection_timeout = options and options.connection_timeout or nil

    local ok, err = pcall(function()
        client_socket = socket.tcp()
        assert(client_socket:settimeout( connection_timeout ))
        assert(client_socket:connect(server_ip, server_port))
        assert(client_socket:settimeout( 0 ))
        client_socket_table = { client_socket }
        send_queue = tcp_send_queue.create(client_socket, M.TCP_SEND_CHUNK_SIZE)
    end)

[lharder]

I use tcp_client.lua on MacOSX.

And yes,I fully agree, introducing that options table with a connection_timeout parameter would be a useful change. I would say it is quite a common problem and a good solution ;o)

[britzl]

I've added an options table with a connection timeout setting