From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 6B1C21F462 for ; Sat, 1 Jun 2019 21:43:47 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] ds: fix and test for FD leaks with kqueue on ->Reset Date: Sat, 1 Jun 2019 21:43:47 +0000 Message-Id: <20190601214347.19526-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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. --- MANIFEST | 1 + lib/PublicInbox/DS.pm | 21 +++++++++-------- t/ds-leak.t | 52 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 9 deletions(-) create mode 100644 t/ds-leak.t diff --git a/MANIFEST b/MANIFEST index 2add154..c1a5d67 100644 --- a/MANIFEST +++ b/MANIFEST @@ -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 c165559..9142f21 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 0000000..9e3243e --- /dev/null +++ b/t/ds-leak.t @@ -0,0 +1,52 @@ +# Copyright (C) 2019 all contributors +# Licensed the same as Danga::Socket (and Perl5) +# License: GPL-1.0+ or Artistic-1.0-Perl +# +# +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; -- EW