From e2dfea8b2c74d94b66efc00c2ccab3df7af750a5 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 26 Sep 2023 21:40:00 +0000 Subject: tcp_listener_stats: always eagerly close sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I just debugged an issue with our system, I was witnessing the number of file descriptor in our process grow at an alarming rate which I mapped to our use of raindrops to report utilisation. For various reasons we don’t call raindrops from a Rack middleware but have one process that monitor the socket continuously, and share that data with the workers. Since we call tcp_listener_stats every seconds in a process that doesn't do much else, GC very rarely triggers if at all which cause `InetDiagSocket` instances to accumulate very quickly. Each of those instances holds a file descriptor. Looking at the raindrops implementation it seems to assume the GC will take care of regularly closing these sockets, but I think it’s a bit too bold of an assumption. [ew: don't close user-passed sockets on exception] Acked-by: Eric Wong --- ext/raindrops/linux_inet_diag.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index 2d4f503..e4050cb 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -636,7 +636,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) switch (TYPE(addrs)) { case T_STRING: rb_hash_aset(rv, addrs, tcp_stats(&args, addrs)); - return rv; + goto out; case T_ARRAY: { long i; long len = RARRAY_LEN(addrs); @@ -645,7 +645,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) VALUE cur = rb_ary_entry(addrs, 0); rb_hash_aset(rv, cur, tcp_stats(&args, cur)); - return rv; + goto out; } for (i = 0; i < len; i++) { union any_addr check; @@ -661,6 +661,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) gen_bytecode_all(&args.iov[2]); break; default: + if (argc < 2) rb_io_close(sock); rb_raise(rb_eArgError, "addr must be an array of strings, a string, or nil"); } @@ -673,6 +674,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) if (RHASH_SIZE(rv) > 1) rb_hash_foreach(rv, drop_placeholders, Qfalse); +out: /* let GC deal with corner cases */ rb_str_resize(buf, 0); if (argc < 2) rb_io_close(sock); -- cgit v1.2.3-24-ge0c7