diff options
author | Eric Wong <e@80x24.org> | 2019-06-01 21:43:47 +0000 |
---|---|---|
committer | Eric Wong <e@80x24.org> | 2019-06-01 21:45:24 +0000 |
commit | 1d94048ba6094b9b3bbefe279e066c3992b30408 (patch) | |
tree | f344baeca7a8e8d1d0d31aa026f0b93efa84de59 | |
parent | f4bcc588998c6c4fcf816d7cac16f00ef47bd256 (diff) | |
download | public-inbox-1d94048ba6094b9b3bbefe279e066c3992b30408.tar.gz |
ds: fix and test for FD leaks with kqueue on ->Reset
Even though we currently don't use it repeatedly, ->Reset should close() kqueue FDs and not cause the process to run out of descriptors. Add a close-on-exec test while we're at it.
-rw-r--r-- | MANIFEST | 1 | ||||
-rw-r--r-- | lib/PublicInbox/DS.pm | 21 | ||||
-rw-r--r-- | t/ds-leak.t | 52 |
3 files changed, 65 insertions, 9 deletions
@@ -181,6 +181,7 @@ t/config_limiter.t t/content_id.t t/convert-compact.t t/data/0001.patch +t/ds-leak.t t/emergency.t t/fail-bin/spamc t/feed.t diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm index c1655591..9142f210 100644 --- a/lib/PublicInbox/DS.pm +++ b/lib/PublicInbox/DS.pm @@ -49,8 +49,8 @@ our ( $HaveKQueue, %DescriptorMap, # fd (num) -> PublicInbox::DS object $Epoll, # Global epoll fd (for epoll mode only) - $KQueue, # Global kqueue fd (for kqueue mode only) - $_io, # IO::Handle for Epoll or KQueue + $KQueue, # Global kqueue fd ref (for kqueue mode only) + $_io, # IO::Handle for Epoll @ToClose, # sockets to close when event loop is done $PostLoopCallback, # subref to call at the end of each loop, if defined (global) @@ -84,9 +84,13 @@ sub Reset { %PLCMap = (); $DoneInit = 0; - POSIX::close($Epoll) if defined $Epoll && $Epoll >= 0; - POSIX::close($KQueue) if defined $KQueue && $KQueue >= 0; - $_io = undef; + # NOTE kqueue is close-on-fork, and we don't account for it, yet + # OTOH, we (public-inbox) don't need this sub outside of tests... + POSIX::close($$KQueue) if !$_io && $KQueue && $$KQueue >= 0; + $KQueue = undef; + + $_io = undef; # close $Epoll + $Epoll = undef; *EventLoop = *FirstTimeEventLoop; } @@ -168,11 +172,11 @@ sub AddTimer { die "Shouldn't get here."; } +# keeping this around in case we support other FD types for now, +# epoll_create1(EPOLL_CLOEXEC) requires Linux 2.6.27+... sub set_cloexec ($) { my ($fd) = @_; - # new_from_fd fails on real kqueue, but is needed for libkqueue - # (which emulates kqueue via epoll) $_io = IO::Handle->new_from_fd($fd, 'r+') or return; defined(my $fl = fcntl($_io, F_GETFD, 0)) or return; fcntl($_io, F_SETFD, $fl | FD_CLOEXEC); @@ -185,9 +189,8 @@ sub _InitPoller if ($HAVE_KQUEUE) { $KQueue = IO::KQueue->new(); - $HaveKQueue = $KQueue >= 0; + $HaveKQueue = defined $KQueue; if ($HaveKQueue) { - set_cloexec($KQueue); # needed if using libkqueue & epoll *EventLoop = *KQueueEventLoop; } } diff --git a/t/ds-leak.t b/t/ds-leak.t new file mode 100644 index 00000000..9e3243e9 --- /dev/null +++ b/t/ds-leak.t @@ -0,0 +1,52 @@ +# Copyright (C) 2019 all contributors <meta@public-inbox.org> +# Licensed the same as Danga::Socket (and Perl5) +# License: GPL-1.0+ or Artistic-1.0-Perl +# <https://www.gnu.org/licenses/gpl-1.0.txt> +# <https://dev.perl.org/licenses/artistic.html> +use strict; +use warnings; +use Test::More; +use_ok 'PublicInbox::DS'; + +subtest('close-on-exec for epoll and kqueue' => sub { + use PublicInbox::Spawn qw(spawn); + my $pid; + my $evfd_re = qr/(?:kqueue|eventpoll)/i; + + PublicInbox::DS->SetLoopTimeout(0); + PublicInbox::DS->SetPostLoopCallback(sub { 0 }); + PublicInbox::DS->AddTimer(0, sub { $pid = spawn([qw(sleep 10)]) }); + PublicInbox::DS->EventLoop; + ok($pid, 'subprocess spawned'); + my @of = grep(/$evfd_re/, `lsof -p $pid 2>/dev/null`); + my $err = $?; + SKIP: { + skip "lsof missing? (\$?=$err)", 1 if $err; + is_deeply(\@of, [], 'no FDs leaked to subprocess'); + }; + if (defined $pid) { + kill(9, $pid); + waitpid($pid, 0); + } + PublicInbox::DS->Reset; +}); + +SKIP: { + # not bothering with BSD::Resource + chomp(my $n = `/bin/sh -c 'ulimit -n'`); + + # FreeBSD 11.2 with 2GB RAM gives RLIMIT_NOFILE=57987! + if ($n > 1024 && !$ENV{TEST_EXPENSIVE}) { + skip "RLIMIT_NOFILE=$n too big w/o TEST_EXPENSIVE for $0", 1; + } + my $cb = sub {}; + for my $i (0..$n) { + PublicInbox::DS->SetLoopTimeout(0); + PublicInbox::DS->SetPostLoopCallback($cb); + PublicInbox::DS->EventLoop; + PublicInbox::DS->Reset; + } + ok(1, "Reset works and doesn't hit RLIMIT_NOFILE ($n)"); +}; + +done_testing; |