about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <e@yhbt.net>2020-07-04 21:33:13 +0000
committerEric Wong <e@yhbt.net>2020-07-05 21:45:19 +0000
commitd75cbb9b69a780b6bbc37e243ada35dfd0c47552 (patch)
tree12fb44285885b52a32772e22a75f2e04fd51541d
parent2d0438df824317049e29837c6d187b54ea89b515 (diff)
downloadpublic-inbox-d75cbb9b69a780b6bbc37e243ada35dfd0c47552.tar.gz
Network connections fail and need to be detected sooner rather
than later during IDLE to avoid backtrace floods.  In case the
IDLE process dies completely, don't respawn right away, either,
to avoid entering a respawn loop.

There's also a typo fix :P
-rw-r--r--lib/PublicInbox/WatchMaildir.pm23
-rw-r--r--t/imapd.t29
2 files changed, 45 insertions, 7 deletions
diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm
index 23b2e9f1..7547f6e4 100644
--- a/lib/PublicInbox/WatchMaildir.pm
+++ b/lib/PublicInbox/WatchMaildir.pm
@@ -497,7 +497,8 @@ sub imap_idle_once ($$$$) {
         }
         $self->{idle_mic} = $mic; # for ->quit
         my @res;
-        until ($self->{quit} || grep(/^\* [0-9]+ EXISTS/, @res) || $i <= 0) {
+        until ($self->{quit} || !$mic->IsConnected ||
+                        grep(/^\* [0-9]+ EXISTS/, @res) || $i <= 0) {
                 @res = $mic->idle_data($i);
                 $i = $end - now();
         }
@@ -520,8 +521,13 @@ sub watch_imap_idle_1 ($$$) {
         local $0 = $uri->mailbox." $sec";
         until ($self->{quit}) {
                 $mic //= PublicInbox::IMAPClient->new(%$mic_arg);
-                my $err = imap_fetch_all($self, $mic, $url);
-                $err //= imap_idle_once($self, $mic, $intvl, $url);
+                my $err;
+                if ($mic && $mic->IsConnected) {
+                        $err = imap_fetch_all($self, $mic, $url);
+                        $err //= imap_idle_once($self, $mic, $intvl, $url);
+                } else {
+                        $err = "not connected: $!";
+                }
                 if ($err && !$self->{quit}) {
                         warn $err, "\n";
                         $mic = undef;
@@ -545,6 +551,13 @@ sub watch_atfork_parent ($) {
         _done_for_now($self);
 }
 
+sub imap_idle_requeue ($) { # DS::add_timer callback
+        my ($self, $url_intvl) = @{$_[0]};
+        return if $self->{quit};
+        push @{$self->{idle_todo}}, $url_intvl;
+        event_step($self);
+}
+
 sub imap_idle_reap { # PublicInbox::DS::dwaitpid callback
         my ($self, $pid) = @_;
         my $url_intvl = delete $self->{idle_pids}->{$pid} or
@@ -553,8 +566,8 @@ sub imap_idle_reap { # PublicInbox::DS::dwaitpid callback
         my ($url, $intvl) = @$url_intvl;
         return if $self->{quit};
         warn "W: PID=$pid on $url died: \$?=$?\n" if $?;
-        push @{$self->{idle_todo}}, $url_intvl;
-        PubicInbox::DS::requeue($self); # call ->event_step to respawn
+        PublicInbox::DS::add_timer(60,
+                                \&imap_idle_requeue, [ $self, $url_intvl ]);
 }
 
 sub imap_idle_fork ($$) {
diff --git a/t/imapd.t b/t/imapd.t
index cf327e9f..1ac6a4ab 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -468,7 +468,7 @@ SKIP: {
         my $obj = bless \$cb, 'PublicInbox::TestCommon::InboxWakeup';
         $cfg->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) });
         my $watcherr = "$tmpdir/watcherr";
-        open my $err_wr, '>', $watcherr or BAIL_OUT $!;
+        open my $err_wr, '>>', $watcherr or BAIL_OUT $!;
         open my $err, '<', $watcherr or BAIL_OUT $!;
         my $w = start_script(['-watch'], undef, { 2 => $err_wr });
 
@@ -512,11 +512,36 @@ SKIP: {
         seek($err, 0, 0);
         my @err = grep(!/^I:/, <$err>);
         is(@err, 0, 'no warnings/errors from -watch'.join(' ', @err));
+
+        if ($ENV{TEST_KILL_IMAPD}) { # not sure how reliable this test can be
+                xsys(qw(git config), "--file=$home/.public-inbox/config",
+                        qw(--unset imap.PollInterval)) == 0
+                        or BAIL_OUT "git config $?";
+                truncate($err_wr, 0) or BAIL_OUT $!;
+                my @t0 = times;
+                $w = start_script(['-watch'], undef, { 2 => $err_wr });
+                seek($err, 0, 0);
+                tick until (grep(/I: \S+ idling/, <$err>));
+                diag 'killing imapd, waiting for CPU spins';
+                my $delay = 0.11;
+                $td->kill(9);
+                tick $delay;
+                $w->kill;
+                $w->join;
+                is($?, 0, 'no error in exited -watch process');
+                my @t1 = times;
+                my $c = $t1[2] + $t1[3] - $t0[2] - $t0[3];
+                my $thresh = (0.9 * $delay);
+                diag "c=$c, threshold=$thresh";
+                ok($c < $thresh, 'did not burn much CPU');
+                is_deeply([grep(/ line \d+$/m, <$err>)], [],
+                                'no backtraces from errors');
+        }
 }
 
 $td->kill;
 $td->join;
-is($?, 0, 'no error in exited process');
+is($?, 0, 'no error in exited process') if !$ENV{TEST_KILL_IMAPD};
 open my $fh, '<', $err or BAIL_OUT("open $err failed: $!");
 my $eout = do { local $/; <$fh> };
 unlike($eout, qr/wide/i, 'no Wide character warnings');