nrk / redis-lua

A Lua client library for the redis key value storage system.
MIT License
731 stars 239 forks source link

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

Open q3k opened 3 years ago

q3k commented 3 years ago

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
soroshsabz commented 2 years ago

@nrk Did you think this issue is important?

narcoticfresh commented 1 year ago

@soroshsabz it is an important issue..