* [PATCH 1/9] doc: lei-security: some more updates
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 2/9] listener: switch to level-triggered epoll Eric Wong
` (7 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
Virtual users will probably be used for read-write IMAP/JMAP
support. The potential for various kernel/hardware bugs and
attacks also needs to be highlighted.
---
Documentation/lei-security.pod | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/lei-security.pod b/Documentation/lei-security.pod
index 02305b9055c2..8cbd89934568 100644
--- a/Documentation/lei-security.pod
+++ b/Documentation/lei-security.pod
@@ -18,6 +18,9 @@ permissions support.
It does not use POSIX ACLs, extended attributes, nor any other
security-related functions which require non-standard Perl modules.
+There is preliminary support for "virtual users", but it is
+incomplete and undocumented.
+
=head1 INTERNAL FILES
lei runs with a umask of 077 to prevent other users on the
@@ -93,7 +96,7 @@ lei uses L<git-credential(1)> to prompt users for IMAP and NNTP
usernames and passwords. These passwords are not encrypted in
memory and get transferred across processes via anonymous UNIX
sockets and pipes. They may be exposed via syscall tracing
-tools (e.g. L<strace(1)>).
+tools (e.g. L<strace(1)>), kernel and hardware bugs/attacks.
While credentials are not written to the filesystem by default,
it is possible for them to end up on disk if processes are
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/9] listener: switch to level-triggered epoll
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
2021-10-01 9:54 ` [PATCH 1/9] doc: lei-security: some more updates Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 3/9] daemon: make SO_ACCEPTFILTER a shared variable Eric Wong
` (6 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
On second thought, the ->requeue + accept retry code path isn't
worth the userspace complexity and overhead. Level-triggered
epoll has always annoyed me since it takes an inefficient code
path in the kernel; but taking our less-efficient code path in
Perl seems even worse. We also need to take load distribution
into account for multi-worker systems.
---
lib/PublicInbox/Listener.pm | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/Listener.pm b/lib/PublicInbox/Listener.pm
index 64bba5b0fd7c..09f1f2e5fabd 100644
--- a/lib/PublicInbox/Listener.pm
+++ b/lib/PublicInbox/Listener.pm
@@ -7,7 +7,7 @@ use strict;
use parent 'PublicInbox::DS';
use Socket qw(SOL_SOCKET SO_KEEPALIVE IPPROTO_TCP TCP_NODELAY);
use IO::Handle;
-use PublicInbox::Syscall qw(EPOLLIN EPOLLEXCLUSIVE EPOLLET);
+use PublicInbox::Syscall qw(EPOLLIN EPOLLEXCLUSIVE);
use Errno qw(EAGAIN ECONNABORTED EPERM);
# Warn on transient errors, mostly resource limitations.
@@ -22,7 +22,7 @@ sub new ($$$) {
setsockopt($s, IPPROTO_TCP, TCP_NODELAY, 1); # ignore errors on non-TCP
listen($s, 2**31 - 1); # kernel will clamp
my $self = bless { post_accept => $cb }, $class;
- $self->SUPER::new($s, EPOLLIN|EPOLLET|EPOLLEXCLUSIVE);
+ $self->SUPER::new($s, EPOLLIN|EPOLLEXCLUSIVE);
}
sub event_step {
@@ -38,7 +38,6 @@ sub event_step {
IO::Handle::blocking($c, 0); # no accept4 :<
eval { $self->{post_accept}->($c, $addr, $sock) };
warn "E: $@\n" if $@;
- $self->requeue;
} elsif ($! == EAGAIN || $! == ECONNABORTED || $! == EPERM) {
# EAGAIN is common and likely
# ECONNABORTED is common with bad connections
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/9] daemon: make SO_ACCEPTFILTER a shared variable
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
2021-10-01 9:54 ` [PATCH 1/9] doc: lei-security: some more updates Eric Wong
2021-10-01 9:54 ` [PATCH 2/9] listener: switch to level-triggered epoll Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 4/9] ipc: run Net::SSLeay::randomize Eric Wong
` (5 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
Constant subroutines use more memory and there's no need to
optimize it for inlining since it's only used at startup.
---
lib/PublicInbox/Daemon.pm | 6 +++---
t/httpd-corner.t | 4 ++--
t/httpd-https.t | 6 ++++--
t/httpd.t | 6 ++++--
t/imapd-tls.t | 7 ++++---
t/nntpd-tls.t | 7 ++++---
6 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 727311a4aa10..24dc7791b43d 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -12,7 +12,6 @@ use IO::Handle; # ->autoflush
use IO::Socket;
use POSIX qw(WNOHANG :signal_h);
use Socket qw(IPPROTO_TCP SOL_SOCKET);
-sub SO_ACCEPTFILTER () { 0x1000 }
STDOUT->autoflush(1);
STDERR->autoflush(1);
use PublicInbox::DS qw(now);
@@ -21,6 +20,7 @@ require PublicInbox::Listener;
use PublicInbox::EOFpipe;
use PublicInbox::Sigfd;
use PublicInbox::GitAsyncCat;
+our $SO_ACCEPTFILTER = 0x1000;
my @CMD;
my ($set_user, $oldset);
my (@cfg_listen, $stdout, $stderr, $group, $user, $pid_file, $daemonize);
@@ -579,10 +579,10 @@ sub defer_accept ($$) {
return if $sec > 0; # systemd users may set a higher value
setsockopt($s, IPPROTO_TCP, $TCP_DEFER_ACCEPT, 1);
} elsif ($^O eq 'freebsd') {
- my $x = getsockopt($s, SOL_SOCKET, SO_ACCEPTFILTER);
+ my $x = getsockopt($s, SOL_SOCKET, $SO_ACCEPTFILTER);
return if defined $x; # don't change if set
my $accf_arg = pack('a16a240', $af_name, '');
- setsockopt($s, SOL_SOCKET, SO_ACCEPTFILTER, $accf_arg);
+ setsockopt($s, SOL_SOCKET, $SO_ACCEPTFILTER, $accf_arg);
}
}
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 5dc5734e96d1..cec754c9c13a 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -36,7 +36,7 @@ if ($^O eq 'linux') {
}
} elsif ($^O eq 'freebsd' && system('kldstat -m accf_data >/dev/null') == 0) {
require PublicInbox::Daemon;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
+ my $var = $PublicInbox::Daemon::SO_ACCEPTFILTER;
$accf_arg = pack('a16a240', 'dataready', '');
setsockopt($sock, SOL_SOCKET, $var, $accf_arg) or die "setsockopt: $!";
}
@@ -596,7 +596,7 @@ SKIP: {
SKIP: {
skip 'SO_ACCEPTFILTER is FreeBSD-only', 1 if $^O ne 'freebsd';
skip 'accf_data not loaded: kldload accf_data' if !defined $accf_arg;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
+ my $var = $PublicInbox::Daemon::SO_ACCEPTFILTER;
defined(my $x = getsockopt($sock, SOL_SOCKET, $var)) or die;
is($x, $accf_arg, 'SO_ACCEPTFILTER unchanged if previously set');
};
diff --git a/t/httpd-https.t b/t/httpd-https.t
index bf7d3f94de6c..d42d7c509949 100644
--- a/t/httpd-https.t
+++ b/t/httpd-https.t
@@ -98,8 +98,10 @@ for my $args (
skip 'accf_data not loaded? kldload accf_data', 2;
}
require PublicInbox::Daemon;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
- my $x = getsockopt($https, SOL_SOCKET, $var);
+ ok(defined($PublicInbox::Daemon::SO_ACCEPTFILTER),
+ 'SO_ACCEPTFILTER defined');
+ my $x = getsockopt($https, SOL_SOCKET,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
like($x, qr/\Adataready\0+\z/, 'got dataready accf for https');
};
diff --git a/t/httpd.t b/t/httpd.t
index 849f61bb82ce..aa6012103141 100644
--- a/t/httpd.t
+++ b/t/httpd.t
@@ -109,8 +109,10 @@ SKIP: {
skip 'accf_http not loaded: kldload accf_http', 1;
}
require PublicInbox::Daemon;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
- my $x = getsockopt($sock, SOL_SOCKET, $var);
+ ok(defined($PublicInbox::Daemon::SO_ACCEPTFILTER),
+ 'SO_ACCEPTFILTER defined');
+ my $x = getsockopt($sock, SOL_SOCKET,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
like($x, qr/\Ahttpready\0+\z/, 'got httpready accf for HTTP');
};
diff --git a/t/imapd-tls.t b/t/imapd-tls.t
index 73f5112fcb72..44ab350c95b3 100644
--- a/t/imapd-tls.t
+++ b/t/imapd-tls.t
@@ -176,10 +176,11 @@ for my $args (
skip 'accf_data not loaded? kldload accf_data', 2;
}
require PublicInbox::Daemon;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
- my $x = getsockopt($imaps, SOL_SOCKET, $var);
+ my $x = getsockopt($imaps, SOL_SOCKET,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
like($x, qr/\Adataready\0+\z/, 'got dataready accf for IMAPS');
- $x = getsockopt($starttls, IPPROTO_TCP, $var);
+ $x = getsockopt($starttls, IPPROTO_TCP,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
is($x, undef, 'no BSD accept filter for plain IMAP');
};
diff --git a/t/nntpd-tls.t b/t/nntpd-tls.t
index 9af6c25443a0..d81d1e1303fc 100644
--- a/t/nntpd-tls.t
+++ b/t/nntpd-tls.t
@@ -168,10 +168,11 @@ for my $args (
skip 'accf_data not loaded? kldload accf_data', 2;
}
require PublicInbox::Daemon;
- my $var = PublicInbox::Daemon::SO_ACCEPTFILTER();
- my $x = getsockopt($nntps, SOL_SOCKET, $var);
+ my $x = getsockopt($nntps, SOL_SOCKET,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
like($x, qr/\Adataready\0+\z/, 'got dataready accf for NNTPS');
- $x = getsockopt($starttls, IPPROTO_TCP, $var);
+ $x = getsockopt($starttls, IPPROTO_TCP,
+ $PublicInbox::Daemon::SO_ACCEPTFILTER);
is($x, undef, 'no BSD accept filter for plain NNTP');
};
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/9] ipc: run Net::SSLeay::randomize
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (2 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 3/9] daemon: make SO_ACCEPTFILTER a shared variable Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 5/9] ds: simplify signalfd use Eric Wong
` (4 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
Currently we don't use OpenSSL from child processes of parents
which use OpenSSL, but we may in the future. So ensure OpenSSL
initializes its PRNG after these forks to avoid one security
pitfall down the line.
---
lib/PublicInbox/IPC.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 3e29def87bf5..205b5b92cf71 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -103,6 +103,7 @@ sub ipc_worker_spawn {
my $pid = fork // die "fork: $!";
if ($pid == 0) {
srand($seed);
+ eval { Net::SSLeay::randomize() };
eval { PublicInbox::DS->Reset };
delete @$self{qw(-wq_s1 -wq_s2 -wq_workers -wq_ppid)};
$w_req = $r_res = undef;
@@ -346,6 +347,7 @@ sub _wq_worker_start ($$$$) {
my $pid = fork // die "fork: $!";
if ($pid == 0) {
srand($seed);
+ eval { Net::SSLeay::randomize() };
undef $bcast1;
eval { PublicInbox::DS->Reset };
delete @$self{qw(-wq_s1 -wq_ppid)};
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/9] ds: simplify signalfd use
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (3 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 4/9] ipc: run Net::SSLeay::randomize Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 6/9] inbox: inline and eliminate git_cleanup Eric Wong
` (3 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
Since signalfd is often combined with our event loop, give it a
convenient API and reduce the code duplication required to use it.
EventLoop is replaced with ::event_loop to allow consistent
parameter passing and avoid needlessly passing the package name
on stack.
We also avoid exporting SFD_NONBLOCK since it's the only flag we
support. There's no sense in having the memory overhead of a
constant function when it's in cold code.
---
lib/PublicInbox/ConfigIter.pm | 2 +-
lib/PublicInbox/DS.pm | 64 ++++++++++++++++++---------------
lib/PublicInbox/DSKQXS.pm | 10 +++---
lib/PublicInbox/Daemon.pm | 14 ++------
lib/PublicInbox/ExtMsg.pm | 2 +-
lib/PublicInbox/ExtSearchIdx.pm | 12 ++-----
lib/PublicInbox/Gcf2Client.pm | 4 +--
lib/PublicInbox/IPC.pm | 3 +-
lib/PublicInbox/LEI.pm | 17 ++-------
lib/PublicInbox/Qspawn.pm | 2 +-
lib/PublicInbox/Sigfd.pm | 10 +++---
lib/PublicInbox/Syscall.pm | 12 +++----
lib/PublicInbox/Watch.pm | 3 +-
script/public-inbox-watch | 9 -----
t/dir_idle.t | 6 ++--
t/ds-leak.t | 4 +--
t/imapd.t | 6 ++--
t/nntpd.t | 2 +-
t/sigfd.t | 7 ++--
t/watch_maildir.t | 2 +-
xt/mem-imapd-tls.t | 6 ++--
xt/net_writer-imap.t | 2 +-
22 files changed, 82 insertions(+), 117 deletions(-)
diff --git a/lib/PublicInbox/ConfigIter.pm b/lib/PublicInbox/ConfigIter.pm
index 24cb09bfdc44..14fcef83f229 100644
--- a/lib/PublicInbox/ConfigIter.pm
+++ b/lib/PublicInbox/ConfigIter.pm
@@ -1,7 +1,7 @@
# Copyright (C) 2020-2021 all contributors <meta@public-inbox.org>
# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-# Intended for PublicInbox::DS->EventLoop in read-only daemons
+# Intended for PublicInbox::DS::event_loop in read-only daemons
# to avoid each_inbox() monopolizing the event loop when hundreds/thousands
# of inboxes are in play.
package PublicInbox::ConfigIter;
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index 37cd6087cafb..ba6c74d0ea97 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -155,13 +155,6 @@ sub _InitPoller
}
}
-=head2 C<< CLASS->EventLoop() >>
-
-Start processing IO events. In most daemon programs this never exits. See
-C<PostLoopCallback> below for how to exit the loop.
-
-=cut
-
sub now () { clock_gettime(CLOCK_MONOTONIC) }
sub next_tick () {
@@ -277,26 +270,41 @@ sub PostEventLoop () {
$PostLoopCallback ? $PostLoopCallback->(\%DescriptorMap) : 1;
}
-sub EventLoop {
- $Epoll //= _InitPoller();
- local $in_loop = 1;
- my @events;
- do {
- my $timeout = RunTimers();
-
- # get up to 1000 events
- epoll_wait($Epoll, 1000, $timeout, \@events);
- for my $fd (@events) {
- # it's possible epoll_wait returned many events, including some at the end
- # that ones in the front triggered unregister-interest actions. if we
- # can't find the %sock entry, it's because we're no longer interested
- # in that event.
-
- # guard stack-not-refcounted w/ Carp + @DB::args
- my $obj = $DescriptorMap{$fd};
- $obj->event_step;
- }
- } while (PostEventLoop());
+# Start processing IO events. In most daemon programs this never exits. See
+# C<PostLoopCallback> for how to exit the loop.
+sub event_loop (;$$) {
+ my ($sig, $oldset) = @_;
+ $Epoll //= _InitPoller();
+ require PublicInbox::Sigfd if $sig;
+ my $sigfd = PublicInbox::Sigfd->new($sig, 1) if $sig;
+ local @SIG{keys %$sig} = values(%$sig) if $sig && !$sigfd;
+ local $SIG{PIPE} = 'IGNORE';
+ if (!$sigfd && $sig) {
+ # wake up every second to accept signals if we don't
+ # have signalfd or IO::KQueue:
+ sig_setmask($oldset);
+ PublicInbox::DS->SetLoopTimeout(1000);
+ }
+ $_[0] = $sigfd = $sig = undef; # $_[0] == sig
+ local $in_loop = 1;
+ my @events;
+ do {
+ my $timeout = RunTimers();
+
+ # get up to 1000 events
+ epoll_wait($Epoll, 1000, $timeout, \@events);
+ for my $fd (@events) {
+ # it's possible epoll_wait returned many events,
+ # including some at the end that ones in the front
+ # triggered unregister-interest actions. if we can't
+ # find the %sock entry, it's because we're no longer
+ # interested in that event.
+
+ # guard stack-not-refcounted w/ Carp + @DB::args
+ my $obj = $DescriptorMap{$fd};
+ $obj->event_step;
+ }
+ } while (PostEventLoop());
}
=head2 C<< CLASS->SetPostLoopCallback( CODEREF ) >>
@@ -326,7 +334,7 @@ sub SetPostLoopCallback {
=head2 C<< CLASS->new( $socket ) >>
Create a new PublicInbox::DS subclass object for the given I<socket> which will
-react to events on it during the C<EventLoop>.
+react to events on it during the C<event_loop>.
This is normally (always?) called from your subclass via:
diff --git a/lib/PublicInbox/DSKQXS.pm b/lib/PublicInbox/DSKQXS.pm
index acc31d9baa22..eccfa56d72cb 100644
--- a/lib/PublicInbox/DSKQXS.pm
+++ b/lib/PublicInbox/DSKQXS.pm
@@ -18,7 +18,7 @@ use Symbol qw(gensym);
use IO::KQueue;
use Errno qw(EAGAIN);
use PublicInbox::Syscall qw(EPOLLONESHOT EPOLLIN EPOLLOUT EPOLLET
- EPOLL_CTL_ADD EPOLL_CTL_MOD EPOLL_CTL_DEL SFD_NONBLOCK);
+ EPOLL_CTL_ADD EPOLL_CTL_MOD EPOLL_CTL_DEL);
our @EXPORT_OK = qw(epoll_ctl epoll_wait);
sub EV_DISPATCH () { 0x0080 }
@@ -48,16 +48,16 @@ sub new {
# It's wasteful in that it uses another FD, but it simplifies
# our epoll-oriented code.
sub signalfd {
- my ($class, $signo, $flags) = @_;
+ my ($class, $signo, $nonblock) = @_;
my $sym = gensym;
- tie *$sym, $class, $signo, $flags; # calls TIEHANDLE
+ tie *$sym, $class, $signo, $nonblock; # calls TIEHANDLE
$sym
}
sub TIEHANDLE { # similar to signalfd()
- my ($class, $signo, $flags) = @_;
+ my ($class, $signo, $nonblock) = @_;
my $self = $class->new;
- $self->{timeout} = ($flags & SFD_NONBLOCK) ? 0 : -1;
+ $self->{timeout} = $nonblock ? 0 : -1;
my $kq = $self->{kq};
$kq->EV_SET($_, EVFILT_SIGNAL, EV_ADD) for @$signo;
$self;
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 24dc7791b43d..5be474fa8754 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -15,7 +15,6 @@ use Socket qw(IPPROTO_TCP SOL_SOCKET);
STDOUT->autoflush(1);
STDERR->autoflush(1);
use PublicInbox::DS qw(now);
-use PublicInbox::Syscall qw(SFD_NONBLOCK);
require PublicInbox::Listener;
use PublicInbox::EOFpipe;
use PublicInbox::Sigfd;
@@ -513,7 +512,7 @@ EOF
},
CHLD => \&reap_children,
};
- my $sigfd = PublicInbox::Sigfd->new($sig, 0);
+ my $sigfd = PublicInbox::Sigfd->new($sig);
local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
PublicInbox::DS::sig_setmask($oldset) if !$sigfd;
while (1) { # main loop
@@ -630,20 +629,11 @@ sub daemon_loop ($$$$) {
# this calls epoll_create:
PublicInbox::Listener->new($_, $tls_cb || $post_accept)
} @listeners;
- my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
- local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
- if (!$sigfd) {
- # wake up every second to accept signals if we don't
- # have signalfd or IO::KQueue:
- PublicInbox::DS::sig_setmask($oldset);
- PublicInbox::DS->SetLoopTimeout(1000);
- }
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop($sig, $oldset);
}
sub run ($$$;$) {
my ($default, $refresh, $post_accept, $tlsd) = @_;
- local $SIG{PIPE} = 'IGNORE';
daemon_prepare($default);
my $af_default = $default =~ /:8080\z/ ? 'httpready' : undef;
my $for_destroy = daemonize();
diff --git a/lib/PublicInbox/ExtMsg.pm b/lib/PublicInbox/ExtMsg.pm
index c134de55d16c..72cae005da5a 100644
--- a/lib/PublicInbox/ExtMsg.pm
+++ b/lib/PublicInbox/ExtMsg.pm
@@ -150,7 +150,7 @@ sub ext_msg {
};
}
-# called via PublicInbox::DS->EventLoop
+# called via PublicInbox::DS::event_loop
sub event_step {
my ($ctx, $sync) = @_;
# can't find a partial match in current inbox, try the others:
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 6b29789a2ed8..c34225b29d9a 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1305,19 +1305,11 @@ sub eidx_watch { # public-inbox-extindex --watch main loop
};
my $quit = PublicInbox::SearchIdx::quit_cb($sync);
$sig->{QUIT} = $sig->{INT} = $sig->{TERM} = $quit;
- my $sigfd = PublicInbox::Sigfd->new($sig,
- $PublicInbox::Syscall::SFD_NONBLOCK);
- @SIG{keys %$sig} = values(%$sig) if !$sigfd;
local $self->{-watch_sync} = $sync; # for ->on_inbox_unlock
- if (!$sigfd) {
- # wake up every second to accept signals if we don't
- # have signalfd or IO::KQueue:
- PublicInbox::DS::sig_setmask($oldset);
- PublicInbox::DS->SetLoopTimeout(1000);
- }
PublicInbox::DS->SetPostLoopCallback(sub { !$sync->{quit} });
$pr->("initial scan complete, entering event loop\n") if $pr;
- PublicInbox::DS->EventLoop; # calls InboxIdle->event_step
+ # calls InboxIdle->event_step:
+ PublicInbox::DS::event_loop($sig, $oldset);
done($self);
}
diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index 397774f90bf2..c5695db140cd 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -57,7 +57,7 @@ sub gcf2_async ($$$;$) {
# ensure PublicInbox::Git::cat_async_step never calls cat_async_retry
sub alternates_changed {}
-# DS->EventLoop will call this
+# DS::event_loop will call this
sub event_step {
my ($self) = @_;
$self->flush_write;
@@ -74,7 +74,7 @@ sub event_step {
sub DESTROY {
my ($self) = @_;
- delete $self->{sock}; # if outside EventLoop
+ delete $self->{sock}; # if outside event_loop
PublicInbox::Git::DESTROY($self);
}
diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 205b5b92cf71..6c189b6410aa 100644
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -251,7 +251,7 @@ sub wq_worker_loop ($$) {
my $wqw = PublicInbox::WQWorker->new($self, $self->{-wq_s2});
PublicInbox::WQWorker->new($self, $bcast2) if $bcast2;
PublicInbox::DS->SetPostLoopCallback(sub { $wqw->{sock} });
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
PublicInbox::DS->Reset;
}
@@ -353,7 +353,6 @@ sub _wq_worker_start ($$$$) {
delete @$self{qw(-wq_s1 -wq_ppid)};
$self->{-wq_worker_nr} =
keys %{delete($self->{-wq_workers}) // {}};
- $SIG{$_} = 'IGNORE' for (qw(PIPE));
$SIG{$_} = 'DEFAULT' for (qw(TTOU TTIN TERM QUIT INT CHLD));
local $0 = $one ? $self->{-wq_ident} :
"$self->{-wq_ident} $self->{-wq_worker_nr}";
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index df0bfab6dfb7..fd59235846ae 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -18,8 +18,7 @@ use POSIX qw(strftime);
use IO::Handle ();
use Fcntl qw(SEEK_SET);
use PublicInbox::Config;
-use PublicInbox::Syscall qw(SFD_NONBLOCK EPOLLIN EPOLLET);
-use PublicInbox::Sigfd;
+use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
use PublicInbox::DS qw(now dwaitpid);
use PublicInbox::Spawn qw(spawn popen_rd);
use PublicInbox::Lock;
@@ -1291,23 +1290,11 @@ sub lazy_start {
USR1 => \&noop,
USR2 => \&noop,
};
- my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
- local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
- undef $sig;
- local $SIG{PIPE} = 'IGNORE';
require PublicInbox::DirIdle;
local $dir_idle = PublicInbox::DirIdle->new([$sock_dir], sub {
# just rely on wakeup to hit PostLoopCallback set below
dir_idle_handler($_[0]) if $_[0]->fullname ne $path;
}, 1);
- if ($sigfd) {
- undef $sigfd; # unref, already in DS::DescriptorMap
- } else {
- # wake up every second to accept signals if we don't
- # have signalfd or IO::KQueue:
- PublicInbox::DS::sig_setmask($oldset);
- PublicInbox::DS->SetLoopTimeout(1000);
- }
PublicInbox::DS->SetPostLoopCallback(sub {
my ($dmap, undef) = @_;
if (@st = defined($path) ? stat($path) : ()) {
@@ -1344,7 +1331,7 @@ sub lazy_start {
open STDERR, '>&STDIN' or die "redirect stderr failed: $!";
open STDOUT, '>&STDIN' or die "redirect stdout failed: $!";
# $daemon pipe to `lei' closed, main loop begins:
- eval { PublicInbox::DS->EventLoop };
+ eval { PublicInbox::DS::event_loop($sig, $oldset) };
warn "event loop error: $@\n" if $@;
# exit() may trigger waitpid via various DESTROY, ensure interruptible
PublicInbox::DS::sig_setmask($oldset);
diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
index 7e50a59ae49e..b1285eda4a83 100644
--- a/lib/PublicInbox/Qspawn.pm
+++ b/lib/PublicInbox/Qspawn.pm
@@ -12,7 +12,7 @@
# operate in. This can be useful to ensure smaller inboxes can
# be cloned while cloning of large inboxes is maxed out.
#
-# This does not depend on the PublicInbox::DS->EventLoop or any
+# This does not depend on the PublicInbox::DS::event_loop or any
# other external scheduling mechanism, you just need to call
# start() and finish() appropriately. However, public-inbox-httpd
# (which uses PublicInbox::DS) will be able to schedule this
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index d91ea0e7ac78..81e5a1b1dd88 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -6,13 +6,13 @@
package PublicInbox::Sigfd;
use strict;
use parent qw(PublicInbox::DS);
-use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET SFD_NONBLOCK);
+use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET);
use POSIX ();
# returns a coderef to unblock signals if neither signalfd or kqueue
# are available.
sub new {
- my ($class, $sig, $flags) = @_;
+ my ($class, $sig, $nonblock) = @_;
my %signo = map {;
my $cb = $sig->{$_};
# SIGWINCH is 28 on FreeBSD, NetBSD, OpenBSD
@@ -24,15 +24,15 @@ sub new {
} keys %$sig;
my $self = bless { sig => \%signo }, $class;
my $io;
- my $fd = signalfd(-1, [keys %signo], $flags);
+ my $fd = signalfd([keys %signo], $nonblock);
if (defined $fd && $fd >= 0) {
open($io, '+<&=', $fd) or die "open: $!";
} elsif (eval { require PublicInbox::DSKQXS }) {
- $io = PublicInbox::DSKQXS->signalfd([keys %signo], $flags);
+ $io = PublicInbox::DSKQXS->signalfd([keys %signo], $nonblock);
} else {
return; # wake up every second to check for signals
}
- if ($flags & SFD_NONBLOCK) { # it can go into the event loop
+ if ($nonblock) { # it can go into the event loop
$self->SUPER::new($io, EPOLLIN | EPOLLET);
} else { # master main loop
$self->{sock} = $io;
diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index a8a6f42a2e2d..7ab4291119ea 100644
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -22,7 +22,7 @@ our @EXPORT_OK = qw(epoll_ctl epoll_create epoll_wait
EPOLLIN EPOLLOUT EPOLLET
EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
EPOLLONESHOT EPOLLEXCLUSIVE
- signalfd SFD_NONBLOCK);
+ signalfd);
our %EXPORT_TAGS = (epoll => [qw(epoll_ctl epoll_create epoll_wait
EPOLLIN EPOLLOUT
EPOLL_CTL_ADD EPOLL_CTL_DEL EPOLL_CTL_MOD
@@ -67,7 +67,6 @@ our (
);
my $SFD_CLOEXEC = 02000000; # Perl does not expose O_CLOEXEC
-sub SFD_NONBLOCK () { O_NONBLOCK }
our $no_deprecated = 0;
if ($^O eq "linux") {
@@ -266,14 +265,15 @@ sub epoll_wait_mod8 {
}
}
-sub signalfd ($$$) {
- my ($fd, $signos, $flags) = @_;
+sub signalfd ($$) {
+ my ($signos, $nonblock) = @_;
if ($SYS_signalfd4) {
my $set = POSIX::SigSet->new(@$signos);
- syscall($SYS_signalfd4, $fd, "$$set",
+ syscall($SYS_signalfd4, -1, "$$set",
# $Config{sig_count} is NSIG, so this is NSIG/8:
int($Config{sig_count}/8),
- $flags|$SFD_CLOEXEC);
+ # SFD_NONBLOCK == O_NONBLOCK for every architecture
+ ($nonblock ? O_NONBLOCK : 0) |$SFD_CLOEXEC);
} else {
$! = ENOSYS;
undef;
diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm
index 0523ad03f871..c6bebce32edb 100644
--- a/lib/PublicInbox/Watch.pm
+++ b/lib/PublicInbox/Watch.pm
@@ -12,7 +12,6 @@ use PublicInbox::MdirReader;
use PublicInbox::NetReader;
use PublicInbox::Filter::Base qw(REJECT);
use PublicInbox::Spamcheck;
-use PublicInbox::Sigfd;
use PublicInbox::DS qw(now add_timer);
use PublicInbox::MID qw(mids);
use PublicInbox::ContentHash qw(content_hash);
@@ -570,7 +569,7 @@ sub watch { # main entry point
}
watch_fs_init($self) if $self->{mdre};
PublicInbox::DS->SetPostLoopCallback(sub { !$self->quit_done });
- PublicInbox::DS->EventLoop; # calls ->event_step
+ PublicInbox::DS::event_loop($sig, $oldset); # calls ->event_step
_done_for_now($self);
}
diff --git a/script/public-inbox-watch b/script/public-inbox-watch
index 86349d71d415..af02d8f358f7 100755
--- a/script/public-inbox-watch
+++ b/script/public-inbox-watch
@@ -13,8 +13,6 @@ use IO::Handle; # ->autoflush
use PublicInbox::Watch;
use PublicInbox::Config;
use PublicInbox::DS;
-use PublicInbox::Sigfd;
-use PublicInbox::Syscall qw(SFD_NONBLOCK);
my $do_scan = 1;
GetOptions('scan!' => \$do_scan, # undocumented, testing only
'help|h' => \(my $show_help)) or do { print STDERR $help; exit 1 };
@@ -56,12 +54,5 @@ if ($watch) {
# --no-scan is only intended for testing atm, undocumented.
PublicInbox::DS::requeue($scan) if $do_scan;
-
- my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
- local @SIG{keys %$sig} = values(%$sig) unless $sigfd;
- if (!$sigfd) {
- PublicInbox::DS::sig_setmask($oldset);
- PublicInbox::DS->SetLoopTimeout(1000);
- }
$watch->watch($sig, $oldset) while ($watch);
}
diff --git a/t/dir_idle.t b/t/dir_idle.t
index 0bb3b7585328..8e7f3b70eec4 100644
--- a/t/dir_idle.t
+++ b/t/dir_idle.t
@@ -15,7 +15,7 @@ my $end = 3 + now;
PublicInbox::DS->SetPostLoopCallback(sub { scalar(@x) == 0 && now < $end });
tick(0.011);
rmdir("$tmpdir/a/b") or xbail "rmdir $!";
-PublicInbox::DS->EventLoop;
+PublicInbox::DS::event_loop();
is(scalar(@x), 1, 'got an event') and
is($x[0]->[0]->fullname, "$tmpdir/a/b", 'got expected fullname') and
ok($x[0]->[0]->IN_DELETE, 'IN_DELETE set');
@@ -24,7 +24,7 @@ tick(0.011);
rmdir("$tmpdir/a") or xbail "rmdir $!";
@x = ();
$end = 3 + now;
-PublicInbox::DS->EventLoop;
+PublicInbox::DS::event_loop();
is(scalar(@x), 1, 'got an event') and
is($x[0]->[0]->fullname, "$tmpdir/a", 'got expected fullname') and
ok($x[0]->[0]->IN_DELETE_SELF, 'IN_DELETE_SELF set');
@@ -33,7 +33,7 @@ tick(0.011);
rename("$tmpdir/c", "$tmpdir/j") or xbail "rmdir $!";
@x = ();
$end = 3 + now;
-PublicInbox::DS->EventLoop;
+PublicInbox::DS::event_loop();
is(scalar(@x), 1, 'got an event') and
is($x[0]->[0]->fullname, "$tmpdir/c", 'got expected fullname') and
ok($x[0]->[0]->IN_DELETE_SELF || $x[0]->[0]->IN_MOVE_SELF,
diff --git a/t/ds-leak.t b/t/ds-leak.t
index 4c211639ed16..4e8d76cdf2ea 100644
--- a/t/ds-leak.t
+++ b/t/ds-leak.t
@@ -19,7 +19,7 @@ if ('close-on-exec for epoll and kqueue') {
pipe($r, $w) or die "pipe: $!";
PublicInbox::DS::add_timer(0, sub { $pid = spawn([qw(sleep 10)]) });
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
ok($pid, 'subprocess spawned');
# wait for execve, we need to ensure lsof sees sleep(1)
@@ -56,7 +56,7 @@ SKIP: {
for my $i (0..$n) {
PublicInbox::DS->SetLoopTimeout(0);
PublicInbox::DS->SetPostLoopCallback($cb);
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
PublicInbox::DS->Reset;
}
ok(1, "Reset works and doesn't hit RLIMIT_NOFILE ($n)");
diff --git a/t/imapd.t b/t/imapd.t
index bd8ad7e5162d..80757a9d4071 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -466,7 +466,7 @@ SKIP: {
my $w = start_script(['-watch'], undef, { 2 => $err_wr });
diag 'waiting for initial fetch...';
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
diag 'inbox unlocked on initial fetch, waiting for IDLE';
tick until (grep(/I: \S+ idling/, <$err>));
@@ -477,7 +477,7 @@ SKIP: {
diag "mda error \$?=$?";
diag 'waiting for IMAP IDLE wakeup';
PublicInbox::DS->SetPostLoopCallback(undef);
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
diag 'inbox unlocked on IDLE wakeup';
# try again with polling
@@ -494,7 +494,7 @@ SKIP: {
diag 'waiting for PollInterval wakeup';
PublicInbox::DS->SetPostLoopCallback(undef);
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
diag 'inbox unlocked (poll)';
$w->kill;
$w->join;
diff --git a/t/nntpd.t b/t/nntpd.t
index 3c171a3b88b9..cf1c44f80b23 100644
--- a/t/nntpd.t
+++ b/t/nntpd.t
@@ -439,7 +439,7 @@ sub test_watch {
my $w = start_script(['-watch'], undef, { 2 => $err_wr });
diag 'waiting for initial fetch...';
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
diag 'inbox unlocked on initial fetch';
$w->kill;
$w->join;
diff --git a/t/sigfd.t b/t/sigfd.t
index a1ab222c0c4d..a68b12a65f01 100644
--- a/t/sigfd.t
+++ b/t/sigfd.t
@@ -4,7 +4,6 @@ use Test::More;
use IO::Handle;
use POSIX qw(:signal_h);
use Errno qw(ENOSYS);
-use PublicInbox::Syscall qw(SFD_NONBLOCK);
require_ok 'PublicInbox::Sigfd';
use PublicInbox::DS;
@@ -40,18 +39,18 @@ SKIP: {
}
$sigfd = undef;
- my $nbsig = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
+ my $nbsig = PublicInbox::Sigfd->new($sig, 1);
ok($nbsig, 'Sigfd->new SFD_NONBLOCK works');
is($nbsig->wait_once, undef, 'nonblocking ->wait_once');
ok($! == Errno::EAGAIN, 'got EAGAIN');
kill('HUP', $$) or die "kill $!";
PublicInbox::DS->SetPostLoopCallback(sub {}); # loop once
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
is($hit->{HUP}->{sigfd}, 2, 'HUP sigfd fired in event loop') or
diag explain($hit); # sometimes fails on FreeBSD 11.x
kill('TERM', $$) or die "kill $!";
kill('HUP', $$) or die "kill $!";
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
PublicInbox::DS->Reset;
is($hit->{TERM}->{sigfd}, 1, 'TERM sigfd fired in event loop');
is($hit->{HUP}->{sigfd}, 3, 'HUP sigfd fired in event loop');
diff --git a/t/watch_maildir.t b/t/watch_maildir.t
index e74b512f2192..6399fb7cc15f 100644
--- a/t/watch_maildir.t
+++ b/t/watch_maildir.t
@@ -199,7 +199,7 @@ More majordomo info at http://vger.kernel.org/majordomo-info.html\n);
$em->commit; # wake -watch up
diag 'waiting for -watch to import new message';
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
$wm->kill;
$wm->join;
$ii->close;
diff --git a/xt/mem-imapd-tls.t b/xt/mem-imapd-tls.t
index bd75ef452984..8992a6fc0d8d 100644
--- a/xt/mem-imapd-tls.t
+++ b/xt/mem-imapd-tls.t
@@ -95,7 +95,7 @@ foreach my $n (1..$nfd) {
# one step through the event loop
# do a little work as we connect:
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
# try not to overflow the listen() backlog:
if (!($n % 128) && $DONE != $n) {
@@ -104,7 +104,7 @@ foreach my $n (1..$nfd) {
PublicInbox::DS->SetPostLoopCallback(sub { $DONE != $n });
# clear the backlog:
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
# resume looping
PublicInbox::DS->SetLoopTimeout(0);
@@ -117,7 +117,7 @@ diag "done?: @".time." $DONE/$nfd";
if ($DONE != $nfd) {
PublicInbox::DS->SetLoopTimeout(-1);
PublicInbox::DS->SetPostLoopCallback(sub { $DONE != $nfd });
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
}
is($nfd, $DONE, "$nfd/$DONE done");
if ($^O eq 'linux' && open(my $f, '<', "/proc/$pid/status")) {
diff --git a/xt/net_writer-imap.t b/xt/net_writer-imap.t
index 41438cf79b15..cb2ea61ff8e3 100644
--- a/xt/net_writer-imap.t
+++ b/xt/net_writer-imap.t
@@ -228,7 +228,7 @@ EOM
$pub_cfg->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) });
my $w = start_script(['-watch'], undef, { 2 => $err_wr });
diag 'waiting for initial fetch...';
- PublicInbox::DS->EventLoop;
+ PublicInbox::DS::event_loop();
my $ibx = $pub_cfg->lookup_name('wtest');
my $mm = $ibx->mm;
ok(defined($mm->num_for('Seen@test.example.com')),
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/9] inbox: inline and eliminate git_cleanup
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (4 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 5/9] ds: simplify signalfd use Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 7/9] inbox: keep DB handles if git processes are live Eric Wong
` (2 subsequent siblings)
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
It was probably incorrect to use from max_git_epoch, and it's
small enough to inline into do_cleanup. We'll also eliminate
the unnecessary deletion of {-altid_map} while we're in the
area, since we no longer cache/memoize that.
Fixes: 7e5cea05f061e757 ("inbox: rewrite cleanup to be more aggressive")
---
lib/PublicInbox/Inbox.pm | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 95467d5ac54b..7c1c3afedf2d 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -10,31 +10,22 @@ use PublicInbox::Eml;
use List::Util qw(max);
use Carp qw(croak);
-sub git_cleanup ($) {
- my ($self) = @_;
- my $git = $self->{git} // return undef;
- # normal inboxes have low startup cost and there may be many, so
- # keep process+pipe counts in check. ExtSearch may have high startup
- # cost (e.g. ->ALL) and but likely one per-daemon, so cleanup only
- # if there's unlinked files
- my $live = $self->isa(__PACKAGE__) ? $git->cleanup(1)
- : $git->cleanup_if_unlinked;
- delete($self->{git}) unless $live;
- $live;
-}
-
# returns true if further checking is required
sub cleanup_shards { $_[0]->{search} ? $_[0]->{search}->cleanup_shards : undef }
sub do_cleanup {
my ($ibx) = @_;
- my $live = git_cleanup($ibx);
+ my $live;
+ if (defined $ibx->{git}) {
+ $live = $ibx->isa(__PACKAGE__) ? $ibx->{git}->cleanup(1)
+ : $ibx->{git}->cleanup_if_unlinked;
+ delete($ibx->{git}) unless $live;
+ }
$ibx->cleanup_shards and $live = 1;
for my $git (@{$ibx->{-repo_objs} // []}) {
$live = 1 if $git->cleanup(1);
}
- delete @$ibx{qw(over mm description cloneurl
- -altid_map -imap_url -nntp_url)};
+ delete(@$ibx{qw(over mm description cloneurl -imap_url -nntp_url)});
PublicInbox::DS::add_uniq_timer($ibx+0, 5, \&do_cleanup, $ibx) if $live;
}
@@ -126,7 +117,7 @@ sub max_git_epoch {
my $cur = $self->{-max_git_epoch};
my $changed;
if (!defined($cur) || ($changed = git($self)->alternates_changed)) {
- git_cleanup($self) if $changed;
+ $self->{git}->cleanup if $changed;
my $gits = "$self->{inboxdir}/git";
if (opendir my $dh, $gits) {
my $max = max(map {
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/9] inbox: keep DB handles if git processes are live
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (5 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 6/9] inbox: inline and eliminate git_cleanup Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 8/9] ds: inline set_cloexec Eric Wong
2021-10-01 9:54 ` [PATCH 9/9] doc: lei-daemon: new manpage Eric Wong
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
Having git processes outlive DB handles is likely to hurt
from a fragmentation perspective if the DB handle needs to
be recreated immediately due to a git->cat_async callback.
So only unref DB handles when we're sure there's no live
git users left, otherwise check the inodes.
We'll also avoid needless localization checks in git->cleanup
and make the return value more obvious since the pid fields are
unconditionally deleted nowadays.
---
lib/PublicInbox/Git.pm | 4 ++--
lib/PublicInbox/Inbox.pm | 18 ++++++++++--------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 97c39aad7468..77783000573f 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -399,16 +399,16 @@ sub async_wait_all ($) {
# returns true if there are pending "git cat-file" processes
sub cleanup {
my ($self, $lazy) = @_;
- local $in_cleanup = 1;
return 1 if $lazy && (scalar(@{$self->{inflight_c} // []}) ||
scalar(@{$self->{inflight} // []}));
+ local $in_cleanup = 1;
delete $self->{async_cat};
async_wait_all($self);
delete $self->{inflight};
delete $self->{inflight_c};
_destroy($self, qw(cat_rbuf in out pid));
_destroy($self, qw(chk_rbuf in_c out_c pid_c err_c));
- defined($self->{pid}) || defined($self->{pid_c});
+ undef;
}
# assuming a well-maintained repo, this should be a somewhat
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index 7c1c3afedf2d..724df50a7134 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -13,6 +13,10 @@ use Carp qw(croak);
# returns true if further checking is required
sub cleanup_shards { $_[0]->{search} ? $_[0]->{search}->cleanup_shards : undef }
+sub check_inodes ($) {
+ for (qw(over mm)) { $_[0]->{$_}->check_inodes if $_[0]->{$_} }
+}
+
sub do_cleanup {
my ($ibx) = @_;
my $live;
@@ -21,11 +25,16 @@ sub do_cleanup {
: $ibx->{git}->cleanup_if_unlinked;
delete($ibx->{git}) unless $live;
}
+ if ($live) {
+ check_inodes($ibx);
+ } else {
+ delete(@$ibx{qw(over mm description cloneurl
+ -imap_url -nntp_url)});
+ }
$ibx->cleanup_shards and $live = 1;
for my $git (@{$ibx->{-repo_objs} // []}) {
$live = 1 if $git->cleanup(1);
}
- delete(@$ibx{qw(over mm description cloneurl -imap_url -nntp_url)});
PublicInbox::DS::add_uniq_timer($ibx+0, 5, \&do_cleanup, $ibx) if $live;
}
@@ -362,13 +371,6 @@ sub unsubscribe_unlock {
delete $self->{unlock_subs}->{$ident};
}
-sub check_inodes ($) {
- my ($self) = @_;
- for (qw(over mm)) {
- $self->{$_}->check_inodes if $self->{$_};
- }
-}
-
# called by inotify
sub on_unlock {
my ($self) = @_;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/9] ds: inline set_cloexec
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (6 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 7/9] inbox: keep DB handles if git processes are live Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-01 9:54 ` [PATCH 9/9] doc: lei-daemon: new manpage Eric Wong
8 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
I'm thinking we can drop support for Linux <2.6.27 soonish and
just use EPOLL_CLOEXEC. Perl without signalfd (or
EVFILT_SIGNAL) is miserable, actually.
---
lib/PublicInbox/DS.pm | 46 ++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/lib/PublicInbox/DS.pm b/lib/PublicInbox/DS.pm
index ba6c74d0ea97..debb777a27e8 100644
--- a/lib/PublicInbox/DS.pm
+++ b/lib/PublicInbox/DS.pm
@@ -44,7 +44,7 @@ my $ToClose; # sockets to close when event loop is done
our (
%DescriptorMap, # fd (num) -> PublicInbox::DS object
$Epoll, # Global epoll fd (or DSKQXS ref)
- $_io, # IO::Handle for Epoll
+ $ep_io, # IO::Handle for Epoll
$PostLoopCallback, # subref to call at the end of each loop, if defined (global)
@@ -78,7 +78,7 @@ sub Reset {
for my $q (@q) { @$q = () }
$EXPMAP = undef;
$wait_pids = $nextq = $ToClose = undef;
- $_io = undef; # closes real $Epoll FD
+ $ep_io = undef; # closes real $Epoll FD
$Epoll = undef; # may call DSKQXS::DESTROY
} while (@Timers || keys(%Stack) || $nextq || $wait_pids ||
$ToClose || keys(%DescriptorMap) ||
@@ -127,32 +127,24 @@ sub add_uniq_timer { # ($name, $secs, $coderef, @args) = @_;
$UniqTimer{$_[0]} //= _add_named_timer(@_);
}
-# 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) = @_;
-
- open($_io, '+<&=', $fd) or return;
- defined(my $fl = fcntl($_io, F_GETFD, 0)) or return;
- fcntl($_io, F_SETFD, $fl | FD_CLOEXEC);
-}
-
# caller sets return value to $Epoll
-sub _InitPoller
-{
- if (PublicInbox::Syscall::epoll_defined()) {
- my $fd = epoll_create();
- set_cloexec($fd) if (defined($fd) && $fd >= 0);
- $fd;
- } else {
- my $cls;
- for (qw(DSKQXS DSPoll)) {
- $cls = "PublicInbox::$_";
- last if eval "require $cls";
- }
- $cls->import(qw(epoll_ctl epoll_wait));
- $cls->new;
- }
+sub _InitPoller () {
+ if (PublicInbox::Syscall::epoll_defined()) {
+ my $fd = epoll_create();
+ die "epoll_create: $!" if $fd < 0;
+ open($ep_io, '+<&=', $fd) or return;
+ my $fl = fcntl($ep_io, F_GETFD, 0);
+ fcntl($ep_io, F_SETFD, $fl | FD_CLOEXEC);
+ $fd;
+ } else {
+ my $cls;
+ for (qw(DSKQXS DSPoll)) {
+ $cls = "PublicInbox::$_";
+ last if eval "require $cls";
+ }
+ $cls->import(qw(epoll_ctl epoll_wait));
+ $cls->new;
+ }
}
sub now () { clock_gettime(CLOCK_MONOTONIC) }
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 9/9] doc: lei-daemon: new manpage
2021-10-01 9:54 [PATCH 0/9] daemon-related things Eric Wong
` (7 preceding siblings ...)
2021-10-01 9:54 ` [PATCH 8/9] ds: inline set_cloexec Eric Wong
@ 2021-10-01 9:54 ` Eric Wong
2021-10-02 0:19 ` Kyle Meyer
8 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2021-10-01 9:54 UTC (permalink / raw)
To: meta
In case users see "lei-daemon" in ps(1) or syslog and wonder.
---
Documentation/lei-daemon.pod | 61 ++++++++++++++++++++++++++++++
Documentation/lei.pod | 9 ++++-
Documentation/lei_design_notes.txt | 2 +-
MANIFEST | 1 +
Makefile.PL | 2 +-
5 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 Documentation/lei-daemon.pod
diff --git a/Documentation/lei-daemon.pod b/Documentation/lei-daemon.pod
new file mode 100644
index 000000000000..1205e974706a
--- /dev/null
+++ b/Documentation/lei-daemon.pod
@@ -0,0 +1,61 @@
+=head1 NAME
+
+lei-daemon - technical information for local email interface daemon
+
+=head1 DESCRIPTION
+
+This documentation is a high-level overview for developers and
+administrators interested in how lei works.
+
+lei-daemon is a background daemon which powers the L<lei(1)>
+command-line tool. It may support virtual users and read-write
+IMAP+JMAP APIs in the future. It is designed to optimize shell
+completion by avoiding module loading costs, monitor Maildirs
+(and in the near future, IMAP folders) for changes.
+
+=head2 worker processes
+
+Most commands cause lei-daemon to L<fork(2)> new worker
+processes to isolate and parallelize work. lei-daemon is
+significantly more aggressive than read-only
+L<public-inbox-daemon(8)> processes with regards to resource use
+since it's not designed to support C10K/C100K scenarios.
+
+=head2 file descriptor passing
+
+FD passing is used to reduce IPC costs for bulk I/O when
+importing large mboxes from stdin and dumping large mboxes
+to stdout.
+
+=head2 SOCK_SEQPACKET
+
+SOCK_SEQPACKET sockets are used for both communicating with
+L<lei(1)> and to internal workers. SOCK_SEQPACKET is guarantee
+reliability (unlike SOCK_DGRAM), allows easy load distribution,
+and saves developers the trouble of maintaining stream parsers.
+
+=head2 File monitoring
+
+Inotify or EVFILT_VNODE is used depending on the platform
+to monitor Maildirs of changes and track keyword changes.
+
+The listen socket at (default: C<$XDG_RUNTIME_DIR/lei/5.seq.sock>)
+is also monitored, and the daemon will automatically shutdown
+if it is unlinked.
+
+=head1 CONTACT
+
+Feedback welcome via plain-text mail to L<mailto:meta@public-inbox.org>
+
+The mail archives are hosted at L<https://public-inbox.org/meta/> and
+L<http://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/meta/>
+
+=head1 COPYRIGHT
+
+Copyright all contributors L<mailto:meta@public-inbox.org>
+
+License: AGPL-3.0+ L<https://www.gnu.org/licenses/agpl-3.0.txt>
+
+=head1 SEE ALSO
+
+L<lei-overview(7)>, L<lei-daemon-kill(1)>, L<lei-daemon-pid(1)>
diff --git a/Documentation/lei.pod b/Documentation/lei.pod
index 3af9e2eeacf7..63d5ee6900a7 100644
--- a/Documentation/lei.pod
+++ b/Documentation/lei.pod
@@ -115,6 +115,13 @@ Other subcommands include
By default storage is located at C<$XDG_DATA_HOME/lei/store>. The
configuration for lei resides at C<$XDG_CONFIG_HOME/lei/config>.
+=head1 ERRORS
+
+Errors and dianostics for interactive commands are reported to
+stderr. Some errors for background tasks are emitted via
+L<syslog(3)> as L<lei-daemon(8)> for the top-level daemon,
+and C<lei/store> for the L<lei-store-format(5)> worker.
+
=head1 CONTACT
Feedback welcome via plain-text mail to L<mailto:meta@public-inbox.org>
@@ -130,4 +137,4 @@ License: AGPL-3.0+ L<https://www.gnu.org/licenses/agpl-3.0.txt>
=head1 SEE ALSO
-L<lei-overview(7)>
+L<lei-overview(7)>, L<lei-daemon(8)>
diff --git a/Documentation/lei_design_notes.txt b/Documentation/lei_design_notes.txt
index f1d2ab6f2169..b5180374ad05 100644
--- a/Documentation/lei_design_notes.txt
+++ b/Documentation/lei_design_notes.txt
@@ -6,7 +6,7 @@ Daemon architecture
The use of a persistent daemon works around slow startup time of
Perl. This is especially important for built-in support for
-shell completion. It will eventually support inotify and EVFILT_VNODE
+shell completion. It attempts to support inotify and EVFILT_VNODE
background monitoring of Maildir keyword changes.
If lei were reimplemented in a language with faster startup
diff --git a/MANIFEST b/MANIFEST
index 3e942855127f..929f5f869c5b 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -27,6 +27,7 @@ Documentation/lei-config.pod
Documentation/lei-convert.pod
Documentation/lei-daemon-kill.pod
Documentation/lei-daemon-pid.pod
+Documentation/lei-daemon.pod
Documentation/lei-edit-search.pod
Documentation/lei-export-kw.pod
Documentation/lei-forget-external.pod
diff --git a/Makefile.PL b/Makefile.PL
index 00a558d12472..c41c408a5704 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -60,7 +60,7 @@ $v->{-m5} = [ qw(public-inbox-config public-inbox-v1-format
$v->{-m7} = [ qw(lei-overview lei-security
public-inbox-overview public-inbox-tuning
public-inbox-glossary) ];
-$v->{-m8} = [ qw(public-inbox-daemon) ];
+$v->{-m8} = [ qw(public-inbox-daemon lei-daemon) ];
my @sections = (1, 5, 7, 8);
$v->{check_80} = [];
$v->{manuals} = [];
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 9/9] doc: lei-daemon: new manpage
2021-10-01 9:54 ` [PATCH 9/9] doc: lei-daemon: new manpage Eric Wong
@ 2021-10-02 0:19 ` Kyle Meyer
2021-10-02 8:08 ` Eric Wong
0 siblings, 1 reply; 13+ messages in thread
From: Kyle Meyer @ 2021-10-02 0:19 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
Eric Wong writes:
> In case users see "lei-daemon" in ps(1) or syslog and wonder.
Thanks, this is very helpful/informative.
> diff --git a/Documentation/lei-daemon.pod b/Documentation/lei-daemon.pod
[...]
> +=head2 SOCK_SEQPACKET
> +
> +SOCK_SEQPACKET sockets are used for both communicating with
> +L<lei(1)> and to internal workers. SOCK_SEQPACKET is guarantee
> +reliability (unlike SOCK_DGRAM), allows easy load distribution,
s/is guarantee/guarantees/ ?
> +and saves developers the trouble of maintaining stream parsers.
> +
> +=head2 File monitoring
Obviously unimportant, but this casing is inconsistent with some of the
titles above (e.g., "file descriptor passing").
> +Inotify or EVFILT_VNODE is used depending on the platform
> +to monitor Maildirs of changes and track keyword changes.
s/of changes/for changes/ ?
> +The listen socket at (default: C<$XDG_RUNTIME_DIR/lei/5.seq.sock>)
I stumbled parsing this because target of "at" is parenthesized. At
least in my head, it'd read fine dropping the "at".
> +is also monitored, and the daemon will automatically shutdown
> +if it is unlinked.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 9/9] doc: lei-daemon: new manpage
2021-10-02 0:19 ` Kyle Meyer
@ 2021-10-02 8:08 ` Eric Wong
2021-10-04 3:16 ` Kyle Meyer
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2021-10-02 8:08 UTC (permalink / raw)
To: Kyle Meyer; +Cc: meta
Thanks, pushed with your Helped-by:.
Btw, did you have time to work on some of the other manpages?
No worries if not, I might take a stab at them sometime in the
next few days. I often realize shortcomings while writing
documentation, and I still need to figure out why some messages
go missing or get duplicated...
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 9/9] doc: lei-daemon: new manpage
2021-10-02 8:08 ` Eric Wong
@ 2021-10-04 3:16 ` Kyle Meyer
0 siblings, 0 replies; 13+ messages in thread
From: Kyle Meyer @ 2021-10-04 3:16 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
Eric Wong writes:
> Btw, did you have time to work on some of the other manpages?
> No worries if not, I might take a stab at them sometime in the
> next few days. [...]
Sorry, I haven't worked on manpages for the new commands. (Actually I
still need to catch up on what the new commands are). I suppose at this
point you might prefer to handle it yourself for a quicker turnaround,
but I think I could find time this week.
^ permalink raw reply [flat|nested] 13+ messages in thread