* Re: [PATCH 11/34] watch: use signalfd for Maildir watching
2020-06-27 19:05 7% ` Kyle Meyer
@ 2020-06-27 22:32 7% ` Eric Wong
0 siblings, 0 replies; 4+ results
From: Eric Wong @ 2020-06-27 22:32 UTC (permalink / raw)
To: Kyle Meyer; +Cc: meta
Kyle Meyer <kyle@kyleam.com> wrote:
> One of these days, I'm going to read through a series on this list and
> make a valuable technical comment :] Until then...
No worries, your contributions are valuable regardless of
whether they're technical or not :>
> Eric Wong writes:
>
> > We can get rid of the janky wannabe
> > self-using-a-directory-instead-of-pipe thing we needed to
> > workaround Filesys::Notify::Simple being blocking.
> >
> > For existing Maildir users, this should be more robust and
> > immune to missed wakeups for to signalfd and kqueue-enabled
>
> s/for to/for/ ?
Thanks, will queue up some fixes along with your other comment.
There's deeper issues with this series w.r.t zombies for
existing Maildir users...
^ permalink raw reply [relevance 7%]
* Re: [PATCH 11/34] watch: use signalfd for Maildir watching
2020-06-27 10:03 3% ` [PATCH 11/34] watch: use signalfd for Maildir watching Eric Wong
@ 2020-06-27 19:05 7% ` Kyle Meyer
2020-06-27 22:32 7% ` Eric Wong
0 siblings, 1 reply; 4+ results
From: Kyle Meyer @ 2020-06-27 19:05 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
One of these days, I'm going to read through a series on this list and
make a valuable technical comment :] Until then...
Eric Wong writes:
> We can get rid of the janky wannabe
> self-using-a-directory-instead-of-pipe thing we needed to
> workaround Filesys::Notify::Simple being blocking.
>
> For existing Maildir users, this should be more robust and
> immune to missed wakeups for to signalfd and kqueue-enabled
s/for to/for/ ?
^ permalink raw reply [relevance 7%]
* [PATCH 11/34] watch: use signalfd for Maildir watching
2020-06-27 10:03 5% [PATCH 00/34] watch: add IMAP and NNTP support Eric Wong
@ 2020-06-27 10:03 3% ` Eric Wong
2020-06-27 19:05 7% ` Kyle Meyer
0 siblings, 1 reply; 4+ results
From: Eric Wong @ 2020-06-27 10:03 UTC (permalink / raw)
To: meta
We can get rid of the janky wannabe
self-using-a-directory-instead-of-pipe thing we needed to
workaround Filesys::Notify::Simple being blocking.
For existing Maildir users, this should be more robust and
immune to missed wakeups for to signalfd and kqueue-enabled
systems; as well as being immune to BOFHs clearing $TMPDIR
and preventing notifications from firing.
The IMAP IDLE code still uses normal Perl signals, so it's still
vulnerable to missed wakeups. That will be addressed in future
commits.
---
lib/PublicInbox/Daemon.pm | 19 ++++------
lib/PublicInbox/Sigfd.pm | 12 +++++-
lib/PublicInbox/WatchMaildir.pm | 67 ++++++++++++++++++---------------
script/public-inbox-watch | 24 +++++++++---
t/watch_maildir.t | 4 +-
5 files changed, 75 insertions(+), 51 deletions(-)
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 2f63bd73b4a..ab0c2226e40 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -18,9 +18,9 @@ use PublicInbox::DS qw(now);
use PublicInbox::Syscall qw(SFD_NONBLOCK);
require PublicInbox::Listener;
require PublicInbox::ParentPipe;
-require PublicInbox::Sigfd;
+use PublicInbox::Sigfd;
my @CMD;
-my ($set_user, $oldset, $newset);
+my ($set_user, $oldset);
my (@cfg_listen, $stdout, $stderr, $group, $user, $pid_file, $daemonize);
my $worker_processes = 1;
my @listeners;
@@ -72,15 +72,10 @@ sub accept_tls_opt ($) {
{ SSL_server => 1, SSL_startHandshake => 0, SSL_reuse_ctx => $ctx };
}
-sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
-
sub daemon_prepare ($) {
my ($default_listen) = @_;
my $listener_names = {}; # sockname => IO::Handle
- $oldset = POSIX::SigSet->new();
- $newset = POSIX::SigSet->new();
- $newset->fillset or die "fillset: $!";
- sig_setmask($newset, $oldset);
+ my $oldset = PublicInbox::Sigfd::block_signals();
@CMD = ($0, @ARGV);
my %opts = (
'l|listen=s' => \@cfg_listen,
@@ -515,7 +510,7 @@ EOF
};
my $sigfd = PublicInbox::Sigfd->new($sig, 0);
local %SIG = (%SIG, %$sig) if !$sigfd;
- sig_setmask($oldset) if !$sigfd;
+ PublicInbox::restore_signals($oldset) if !$sigfd;
while (1) { # main loop
my $n = scalar keys %pids;
unless (@listeners) {
@@ -531,7 +526,7 @@ EOF
}
my $want = $worker_processes - 1;
if ($n <= $want) {
- sig_setmask($newset) if !$sigfd;
+ PublicInbox::Sigfd::block_signals() if !$sigfd;
for my $i ($n..$want) {
my $pid = fork;
if (!defined $pid) {
@@ -544,7 +539,7 @@ EOF
$pids{$pid} = $i;
}
}
- sig_setmask($oldset) if !$sigfd;
+ PubliInbox::Sigfd::set_sigmask($oldset) if !$sigfd;
}
if ($sigfd) { # Linux and IO::KQueue users:
@@ -632,7 +627,7 @@ sub daemon_loop ($$$$) {
if (!$sigfd) {
# wake up every second to accept signals if we don't
# have signalfd or IO::KQueue:
- sig_setmask($oldset);
+ PublicInbox::Sigfd::set_sigmask($oldset);
PublicInbox::DS->SetLoopTimeout(1000);
}
PublicInbox::DS->EventLoop;
diff --git a/lib/PublicInbox/Sigfd.pm b/lib/PublicInbox/Sigfd.pm
index f500902ea67..17456592a7e 100644
--- a/lib/PublicInbox/Sigfd.pm
+++ b/lib/PublicInbox/Sigfd.pm
@@ -5,7 +5,7 @@ use strict;
use parent qw(PublicInbox::DS);
use fields qw(sig); # hashref similar to %SIG, but signal numbers as keys
use PublicInbox::Syscall qw(signalfd EPOLLIN EPOLLET SFD_NONBLOCK);
-use POSIX ();
+use POSIX qw(:signal_h);
use IO::Handle ();
# returns a coderef to unblock signals if neither signalfd or kqueue
@@ -62,4 +62,14 @@ sub event_step {
while (wait_once($_[0])) {} # non-blocking
}
+sub sig_setmask { sigprocmask(SIG_SETMASK, @_) or die "sigprocmask: $!" }
+
+sub block_signals () {
+ my $oldset = POSIX::SigSet->new;
+ my $newset = POSIX::SigSet->new;
+ $newset->fillset or die "fillset: $!";
+ sig_setmask($newset, $oldset);
+ $oldset;
+}
+
1;
diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 22f190366a4..4d3cd032e5a 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -8,9 +8,9 @@ use strict;
use warnings;
use PublicInbox::Eml;
use PublicInbox::InboxWritable;
-use File::Temp 0.19 (); # 0.19 for ->newdir
use PublicInbox::Filter::Base qw(REJECT);
use PublicInbox::Spamcheck;
+use PublicInbox::Sigfd;
use PublicInbox::DS qw(now);
use POSIX qw(_exit WNOHANG);
*mime_from_path = \&PublicInbox::InboxWritable::mime_from_path;
@@ -108,6 +108,7 @@ sub new {
imap => scalar keys %imap ? \%imap : undef,
importers => {},
opendirs => {}, # dirname => dirhandle (in progress scans)
+ ops => [], # 'quit', 'full'
}, $class;
}
@@ -195,7 +196,9 @@ sub _try_path {
sub quit {
my ($self) = @_;
- trigger_scan($self, 'quit') or $self->{quit} = 1;
+ $self->{quit} = 1;
+ %{$self->{opendirs}} = ();
+ _done_for_now($self);
if (my $imap_pid = $self->{-imap_pid}) {
kill('QUIT', $imap_pid);
}
@@ -213,24 +216,15 @@ sub quit {
sub watch_fs {
my ($self) = @_;
require PublicInbox::DirIdle;
- my $scan = File::Temp->newdir("public-inbox-watch.$$.scan.XXXXXX",
- TMPDIR => 1);
- my $scandir = $self->{scandir} = $scan->dirname;
- my $scan_re = qr!\A$scandir/!;
my $done = sub {
delete $self->{done_timer};
_done_for_now($self);
};
my $cb = sub {
- my $path = $_[0]->fullname;
- if ($path =~ $scan_re) {
- scan($self, $path);
- } else {
- _try_path($self, $path);
- }
+ _try_path($self, $_[0]->fullname);
$self->{done_timer} //= PublicInbox::DS::requeue($done);
};
- my $di = PublicInbox::DirIdle->new([@{$self->{mdir}}, $scandir], $cb);
+ my $di = PublicInbox::DirIdle->new($self->{mdir}, $cb);
PublicInbox::DS->SetPostLoopCallback(sub { !$self->{quit} });
PublicInbox::DS->EventLoop;
_done_for_now($self);
@@ -485,6 +479,12 @@ sub watch_imap_idle_1 ($$$) {
}
}
+sub watch_atfork_child ($) {
+ my ($self) = @_;
+ PublicInbox::Sigfd::sig_setmask($self->{oldset});
+ %SIG = (%SIG, %{$self->{sig}});
+}
+
sub watch_imap_idle_all ($$) {
my ($self, $idle) = @_; # $idle = [[ uri1, intvl1 ], [ uri2, intvl2 ]]
$self->{mics} = {}; # going to be forking, so disconnect
@@ -494,6 +494,7 @@ sub watch_imap_idle_all ($$) {
my ($uri, $intvl) = @$uri_intvl;
defined(my $pid = fork) or die "fork: $!";
if ($pid == 0) {
+ watch_atfork_child($self);
delete $self->{idle_pids};
watch_imap_idle_1($self, $uri, $intvl);
_exit(0);
@@ -564,15 +565,20 @@ sub watch_imap ($) {
}
sub watch {
- my ($self) = @_;
+ my ($self, $sig, $oldset) = @_;
+ $self->{oldset} = $oldset;
+ $self->{sig} = $sig;
if ($self->{mdre} && $self->{imap}) {
defined(my $pid = fork) or die "fork: $!";
if ($pid == 0) {
+ watch_atfork_child($self);
imap_start($self);
goto &watch_imap;
}
$self->{-imap_pid} = $pid;
} elsif ($self->{imap}) {
+ # not a child process, but no signalfd, yet:
+ watch_atfork_child($self);
imap_start($self);
goto &watch_imap;
}
@@ -580,23 +586,18 @@ sub watch {
}
sub trigger_scan {
- my ($self, $base) = @_;
- my $dir = $self->{scandir} or return;
- open my $fh, '>', "$dir/$base" or die "open $dir/$base failed: $!\n";
- close $fh or die "close $dir/$base failed: $!\n";
+ my ($self, $op) = @_;
+ push @{$self->{ops}}, $op;
+ PublicInbox::DS::requeue($self);
}
-sub scan {
- my ($self, $path) = @_;
- if ($path =~ /quit\z/) {
- %{$self->{opendirs}} = ();
- _done_for_now($self);
- delete $self->{scandir};
- $self->{quit} = 1;
- return;
- }
- # else: $path =~ /(cont|full)\z/
+# called directly, and by PublicInbox::DS
+sub event_step ($) {
+ my ($self) = @_;
return if $self->{quit};
+ my $op = shift @{$self->{ops}};
+
+ # continue existing scan
my $max = 10;
my $opendirs = $self->{opendirs};
my @dirnames = keys %$opendirs;
@@ -609,7 +610,7 @@ sub scan {
}
$opendirs->{$dir} = $dh if $n < 0;
}
- if ($path =~ /full\z/) {
+ if ($op && $op eq 'full') {
foreach my $dir (@{$self->{mdir}}) {
next if $opendirs->{$dir}; # already in progress
my $ok = opendir(my $dh, $dir);
@@ -627,7 +628,13 @@ sub scan {
}
_done_for_now($self);
# do we have more work to do?
- trigger_scan($self, 'cont') if keys %$opendirs;
+ PublicInbox::DS::requeue($self) if keys %$opendirs;
+}
+
+sub scan {
+ my ($self, $op) = @_;
+ push @{$self->{ops}}, $op;
+ goto &event_step;
}
sub _importer_for {
diff --git a/script/public-inbox-watch b/script/public-inbox-watch
index 2057066a2a9..b6d545adad7 100755
--- a/script/public-inbox-watch
+++ b/script/public-inbox-watch
@@ -5,6 +5,10 @@ use strict;
use warnings;
use PublicInbox::WatchMaildir;
use PublicInbox::Config;
+use PublicInbox::DS;
+use PublicInbox::Sigfd;
+use PublicInbox::Syscall qw(SFD_NONBLOCK);
+my $oldset = PublicInbox::Sigfd::block_signals();
my ($config, $watch_md);
my $reload = sub {
$config = PublicInbox::Config->new;
@@ -14,14 +18,22 @@ my $reload = sub {
$reload->();
if ($watch_md) {
my $scan = sub { $watch_md->trigger_scan('full') if $watch_md };
- $SIG{HUP} = $reload;
- $SIG{USR1} = $scan;
- $SIG{ALRM} = sub { $SIG{ALRM} = 'DEFAULT'; $scan->() };
- $SIG{QUIT} = $SIG{TERM} = $SIG{INT} = sub {
+ my $quit = sub {
$watch_md->quit if $watch_md;
$watch_md = undef;
};
+ my $sig = { HUP => $reload, USR1 => $scan };
+ $sig->{QUIT} = $sig->{TERM} = $sig->{INT} = $quit;
+
# --no-scan is only intended for testing atm, undocumented.
- alarm(1) unless (grep(/\A--no-scan\z/, @ARGV));
- $watch_md->watch while ($watch_md);
+ unless (grep(/\A--no-scan\z/, @ARGV)) {
+ PublicInbox::DS::requeue($scan);
+ }
+ my $sigfd = PublicInbox::Sigfd->new($sig, SFD_NONBLOCK);
+ local %SIG = (%SIG, %$sig) if !$sigfd;
+ if (!$sigfd) {
+ PublicInbox::Sigfd::set_sigmask($oldset);
+ PublicInbox::DS->SetLoopTimeout(1000);
+ }
+ $watch_md->watch($sig, $oldset) while ($watch_md);
}
diff --git a/t/watch_maildir.t b/t/watch_maildir.t
index a2c09b0351b..c8658140cf2 100644
--- a/t/watch_maildir.t
+++ b/t/watch_maildir.t
@@ -184,10 +184,10 @@ More majordomo info at http://vger.kernel.org/majordomo-info.html\n);
my $ino_fdinfo = "/proc/$wm->{pid}/fdinfo/$ino_fd";
while (time < $end && open(my $fh, '<', $ino_fdinfo)) {
@ino_info = grep(/^inotify wd:/, <$fh>);
- last if @ino_info >= 4;
+ last if @ino_info >= 3;
tick;
}
- $sleep = undef if @ino_info >= 4;
+ $sleep = undef if @ino_info >= 3;
}
}
if ($sleep) {
^ permalink raw reply related [relevance 3%]
* [PATCH 00/34] watch: add IMAP and NNTP support
@ 2020-06-27 10:03 5% Eric Wong
2020-06-27 10:03 3% ` [PATCH 11/34] watch: use signalfd for Maildir watching Eric Wong
0 siblings, 1 reply; 4+ results
From: Eric Wong @ 2020-06-27 10:03 UTC (permalink / raw)
To: meta
Some fairly major changes to -watch. Filesys::Notify::Simple is
no longer used, and -watch now uses inotify, signalfd or kevent
like the read-only daemons.
Credentials are handled via Net::Netrc (Perl standard library)
or "git-credential", so we do no password storage on our own.
NNTP (and non-IDLE IMAP) may allow more parallelization in the
future.
One significant project-wide change is getting rid of "use
fields". It gets in my way more than it helps, and it's
probably alien to a fair amount of Perl hackers. AFAIK, it's
never really been popular outside of Danga::Socket-based
projects.
Eric W. Biederman (1):
IMAPTracker: Add a helper to track our place in reading imap mailboxes
Eric Wong (33):
inboxwritable: ensure ssoma.lock exists on init
inbox: warn on ->on_inbox_unlock exception
imaptracker: use ~/.local/share/public-inbox/imap.sqlite3
watchmaildir: hoist out compile_watchheaders
watchmaildir: fix check for spam vs ham inbox conflicts
URI IMAP support
watch: preliminary IMAP support
kqnotify|fake_inotify: detect Maildir write ops
watch: remove Filesys::Notify::Simple dependency
watch: use signalfd for Maildir watching
ds: remove fields.pm usage
watch: wire up IMAP IDLE reapers to DS
watch: support IMAP polling
config: support ->urlmatch method for -watch
watch: stop importers before forking
watch: use UID SEARCH to avoid empty UID FETCH
ds: add_timer: allow passing arg to callback.
imaptracker: add {url} field to reduce args
imaptracker: drop {dbname} field
watch: avoid long transaction when writing to IMAPTracker
watch: support imap.fetchBatchSize parameter
watch: imap: be quieter about disconnecting on quit
watch: support multiple watch: directives per-inbox
watch: remove {mdir} array
watch: just use ->urlmatch
testcommon: $ENV{TAIL} supports non-@ARGV redirects
watch: add NNTP support
watch: show user-specified URL consistently.
watch: enable autoflush for STDOUT and STDERR
watch: use our own "git credential" wrapper
watch: support ~/.netrc via Net::Netrc
imaptracker: use flock(2) around writes
watch: simplify internal structures
Documentation/public-inbox-watch.pod | 3 +-
INSTALL | 8 -
MANIFEST | 11 +
Makefile.PL | 4 -
ci/deps.perl | 1 -
lib/PublicInbox/Config.pm | 21 +-
lib/PublicInbox/DS.pm | 29 +-
lib/PublicInbox/Daemon.pm | 19 +-
lib/PublicInbox/DirIdle.pm | 49 ++
lib/PublicInbox/FakeInotify.pm | 56 +-
lib/PublicInbox/GitAsyncCat.pm | 4 +-
lib/PublicInbox/GitCredential.pm | 55 ++
lib/PublicInbox/HTTP.pm | 23 +-
lib/PublicInbox/HTTPD/Async.pm | 22 +-
lib/PublicInbox/IMAP.pm | 19 +-
lib/PublicInbox/IMAPTracker.pm | 82 +++
lib/PublicInbox/In2Tie.pm | 13 +
lib/PublicInbox/Inbox.pm | 1 +
lib/PublicInbox/InboxIdle.pm | 20 +-
lib/PublicInbox/InboxWritable.pm | 3 +
lib/PublicInbox/KQNotify.pm | 38 +-
lib/PublicInbox/Listener.pm | 8 +-
lib/PublicInbox/NNTP.pm | 12 +-
lib/PublicInbox/NNTPdeflate.pm | 5 +-
lib/PublicInbox/ParentPipe.pm | 8 +-
lib/PublicInbox/Sigfd.pm | 21 +-
lib/PublicInbox/TestCommon.pm | 40 +-
lib/PublicInbox/URIimap.pm | 113 +++
lib/PublicInbox/WatchMaildir.pm | 998 +++++++++++++++++++++++----
script/public-inbox-watch | 33 +-
t/config.t | 18 +
t/dir_idle.t | 6 +
t/fake_inotify.t | 45 ++
t/imap_tracker.t | 54 ++
t/imapd.t | 74 ++
t/kqnotify.t | 41 ++
t/nntpd.t | 52 ++
t/uri_imap.t | 65 ++
t/watch_filter_rubylang.t | 2 +-
t/watch_imap.t | 21 +
t/watch_maildir.t | 96 ++-
t/watch_maildir_v2.t | 4 +-
t/watch_multiple_headers.t | 2 +-
t/watch_nntp.t | 17 +
xt/mem-imapd-tls.t | 18 +-
45 files changed, 1944 insertions(+), 290 deletions(-)
create mode 100644 lib/PublicInbox/DirIdle.pm
create mode 100644 lib/PublicInbox/GitCredential.pm
create mode 100644 lib/PublicInbox/IMAPTracker.pm
create mode 100644 lib/PublicInbox/URIimap.pm
create mode 100644 t/dir_idle.t
create mode 100644 t/fake_inotify.t
create mode 100644 t/imap_tracker.t
create mode 100644 t/kqnotify.t
create mode 100644 t/uri_imap.t
create mode 100644 t/watch_imap.t
create mode 100644 t/watch_nntp.t
^ permalink raw reply [relevance 5%]
Results 1-4 of 4 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2020-06-27 10:03 5% [PATCH 00/34] watch: add IMAP and NNTP support Eric Wong
2020-06-27 10:03 3% ` [PATCH 11/34] watch: use signalfd for Maildir watching Eric Wong
2020-06-27 19:05 7% ` Kyle Meyer
2020-06-27 22:32 7% ` Eric Wong
Code repositories for project(s) associated with this public inbox
https://80x24.org/public-inbox.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).