nrk/redis-lua

Socket FD leak on :quit()/:shutdown()

q3k opened this issue · 2 comments

q3k commented

The two methods on the client that permit 'disconnecting' from redis are :quit() and :shutdown(). However, both of them only call :shutdown() on the underlying luasocket socket:

client_prototype.quit = function(client)
    request.multibulk(client, 'QUIT')
    client.network.socket:shutdown()
    return true
end

client_prototype.shutdown = function(client)
    request.multibulk(client, 'SHUTDOWN')
    client.network.socket:shutdown()
end

This causes a socket shutdown - but at least on Linux that is not enough to also close the file descriptor used by the socket. This in turn means that every connection to Redis, even if closed via :close() or :shutdown(), leaks a file descriptor.

One way to fix this would be to call :close() on the underlying luasocket instead of :shutdown() in client:quit and client:shutdown.

Here's an strace of a Lua program using redis-lua opening a Redis connection, calling :quit() on the client, then reopening another connection, showing the FD leak:

socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 14 <-- first connection
fcntl(14, F_GETFL)                      = 0x2 (flags O_RDWR)
fcntl(14, F_SETFL, O_RDWR|O_NONBLOCK)   = 0
connect(14, {sa_family=AF_INET, sin_port=htons(6379), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
poll([{fd=14, events=POLLIN|POLLOUT}], 1, -1) = 1 ([{fd=14, revents=POLLOUT}])
setsockopt(14, SOL_TCP, TCP_NODELAY, [1], 4) = 0
sendto(14, "*2\r\n$4\r\nAUTH\r\n$24\r\nxxxxxxxxxxxxx"..., 45, 0, NULL, 0) = 45
recvfrom(14, 0x4119beb0, 8192, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=14, events=POLLIN}], 1, -1)   = 1 ([{fd=14, revents=POLLIN}])
recvfrom(14, "+OK\r\n", 8192, 0, NULL, NULL) = 5
sendto(14, "*3\r\n$9\r\nRPOPLPUSH\r\n$19\r\nnotifica"..., 76, 0, NULL, 0) = 76
recvfrom(14, "$-1\r\n", 8192, 0, NULL, NULL) = 5
sendto(14, "*1\r\n$4\r\nQUIT\r\n", 14, 0, NULL, 0) = 14
shutdown(14, SHUT_RDWR)  <-- shutdown(2) from luasocket :shutdown() from redis-lua :quit(), missing close(2)
[...]
socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 15 <--- second connection, FD 14 is leaked

@nrk Did you think this issue is important?

@soroshsabz it is an important issue..