luvit / lit

Toolkit for developing, sharing, and running luvit/lua programs and libraries.
http://lit.luvit.io/
Apache License 2.0
245 stars 58 forks source link

coro-net: fix handle leak when connect errors #319

Closed Bilal2453 closed 1 year ago

Bilal2453 commented 1 year ago

Coro-net creates a new handle (whether tcp socket or a pipe) then proceeds to try connecting it. The code handling the connection failure does not seem to free the recently opened handle leading to a leak happening on socket:connect failures.

For ease of testing, I simulated a network error by simply commenting out uv.getaddrinfo and forcing it to connect to a non-existence host / port, forcing a ECONNREFUSED error like so:

  options.isTcp = true
  if options.isTcp then
    local res = {{addr = '127.0.0.1', port = 8000}}
    if not res then return nil, err end
    socket = uv.new_tcp()
    socket:connect(res[1].addr, res[1].port, makeCallback(options.timeout))
    ...

The results of doing multiple coro_net.connect calls with this simulation resulted in the following: image

With this patch it seems to behave: image

One thing I am not entirely sure about if it is possible for socket to not be nil, yet manage to fail on socket:close(), I assume that is not a possible scenario.

Bilal2453 commented 1 year ago

I forgot to include that second close in the first commit, but I also just noticed we only really need socket:close(), no need for the if statement? should I change that?

squeek502 commented 1 year ago

Thanks!