Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
q3k opened this issue Jan 26, 2021 · 2 comments
Open

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

q3k opened this issue Jan 26, 2021 · 2 comments

Comments

@q3k
Copy link

q3k commented Jan 26, 2021

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
Copy link

@nrk Did you think this issue is important?

@narcoticfresh
Copy link

@soroshsabz it is an important issue..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants