about summary refs log tree commit homepage
path: root/lib
diff options
context:
space:
mode:
authorEric Wong <e@yhbt.net>2020-06-10 07:04:04 +0000
committerEric Wong <e@yhbt.net>2020-06-13 07:55:45 +0000
commit9d154055ec44903052beaa4e2c1221f39d6d507a (patch)
treeb76b693d85976839bbedba525a38f99a85760dd1 /lib
parenteab4dfdda4eeea9a54aa674510fa11789c5f91c8 (diff)
downloadpublic-inbox-9d154055ec44903052beaa4e2c1221f39d6d507a.tar.gz
InboxIdle should not be holding onto Inbox objects after the
Config object they came from expires, and Config objects may
expire on SIGHUP.

Old Inbox objects still persist due to IMAP clients holding onto
them, but that's a concern we'll deal with at another time, or
not at all, since all clients expire, eventually.

Regardless, stale inotify watch descriptors should not be left
hanging after SIGHUP refreshes.
Diffstat (limited to 'lib')
-rw-r--r--lib/PublicInbox/IMAP.pm1
-rw-r--r--lib/PublicInbox/IMAPD.pm14
-rw-r--r--lib/PublicInbox/InboxIdle.pm36
3 files changed, 40 insertions, 11 deletions
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 4a43185c..c8592dc0 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -160,6 +160,7 @@ sub cmd_idle ($$) {
         # IDLE seems allowed by dovecot w/o a mailbox selected *shrug*
         my $ibx = $self->{ibx} or return "$tag BAD no mailbox selected\r\n";
         $ibx->subscribe_unlock(fileno($self->{sock}), $self);
+        $self->{imapd}->idler_start;
         $self->{-idle_tag} = $tag;
         $self->{-idle_max} = $ibx->mm->max // 0;
         "+ idling\r\n"
diff --git a/lib/PublicInbox/IMAPD.pm b/lib/PublicInbox/IMAPD.pm
index 1922c160..05aa30e4 100644
--- a/lib/PublicInbox/IMAPD.pm
+++ b/lib/PublicInbox/IMAPD.pm
@@ -16,18 +16,22 @@ sub new {
                 out => \*STDOUT,
                 grouplist => [],
                 # accept_tls => { SSL_server => 1, ..., SSL_reuse_ctx => ... }
+                # pi_config => PublicInbox::Config
                 # idler => PublicInbox::InboxIdle
         }, $class;
 }
 
 sub refresh_groups {
         my ($self) = @_;
-        if (my $old_idler = delete $self->{idler}) {
-                $old_idler->close; # PublicInbox::DS::close
-        }
-        my $pi_config = PublicInbox::Config->new;
-        $self->{idler} = PublicInbox::InboxIdle->new($pi_config);
+        my $pi_config = $self->{pi_config} = PublicInbox::Config->new;
         $self->SUPER::refresh_groups($pi_config);
+        if (my $idler = $self->{idler}) {
+                $idler->refresh($pi_config);
+        }
+}
+
+sub idler_start {
+        $_[0]->{idler} //= PublicInbox::InboxIdle->new($_[0]->{pi_config});
 }
 
 1;
diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm
index 095a801c..c19b8d18 100644
--- a/lib/PublicInbox/InboxIdle.pm
+++ b/lib/PublicInbox/InboxIdle.pm
@@ -4,7 +4,8 @@
 package PublicInbox::InboxIdle;
 use strict;
 use base qw(PublicInbox::DS);
-use fields qw(pi_config inot);
+use fields qw(pi_config inot pathmap);
+use Cwd qw(abs_path);
 use Symbol qw(gensym);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 my $IN_CLOSE = 0x08 | 0x10; # match Linux inotify
@@ -19,13 +20,35 @@ if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) {
 require PublicInbox::In2Tie if $ino_cls;
 
 sub in2_arm ($$) { # PublicInbox::Config::each_inbox callback
-        my ($ibx, $inot) = @_;
-        my $path = "$ibx->{inboxdir}/";
-        $path .= $ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock';
-        $inot->watch($path, $IN_CLOSE, sub { $ibx->on_unlock });
+        my ($ibx, $self) = @_;
+        my $dir = abs_path($ibx->{inboxdir});
+        if (!defined($dir)) {
+                warn "W: $ibx->{inboxdir} not watched: $!\n";
+                return;
+        }
+        my $inot = $self->{inot};
+        my $cur = $self->{pathmap}->{$dir} //= [];
+
+        # transfer old subscriptions to the current inbox, cancel the old watch
+        if (my $old_ibx = $cur->[0]) {
+                $ibx->{unlock_subs} and
+                        die "BUG: $dir->{unlock_subs} should not exist";
+                $ibx->{unlock_subs} = $old_ibx->{unlock_subs};
+                $cur->[1]->cancel;
+        }
+        $cur->[0] = $ibx;
+
+        my $lock = "$dir/".($ibx->version >= 2 ? 'inbox.lock' : 'ssoma.lock');
+        $cur->[1] = $inot->watch($lock, $IN_CLOSE, sub { $ibx->on_unlock });
+
         # TODO: detect deleted packs (and possibly other files)
 }
 
+sub refresh {
+        my ($self, $pi_config) = @_;
+        $pi_config->each_inbox(\&in2_arm, $self);
+}
+
 sub new {
         my ($class, $pi_config) = @_;
         my $self = fields::new($class);
@@ -42,7 +65,8 @@ sub new {
                 $inot = PublicInbox::FakeInotify->new;
         }
         $self->{inot} = $inot;
-        $pi_config->each_inbox(\&in2_arm, $inot);
+        $self->{pathmap} = {}; # inboxdir => [ ibx, watch1, watch2, watch3...]
+        refresh($self, $pi_config);
         $self;
 }