From 39b7af9565f85a720e7eeb7564cfa661000cb7e9 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 3 Apr 2021 02:24:23 +0000 Subject: lei q: ensure wq workers shutdown on IMAP auth failures Leaving workers running on after auth failures is bad and messy, cleanup our process management to have consistent worker teardowns. Improve error reporting, too, instead of letting Mail::IMAPClient->exists fail due to undef. --- lib/PublicInbox/LEI.pm | 24 ++++++++---------------- lib/PublicInbox/LeiAuth.pm | 15 +++++++++------ lib/PublicInbox/NetReader.pm | 2 +- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index f9361c68..cdb4b347 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -350,6 +350,11 @@ my %CONFIG_KEYS = ( my @WQ_KEYS = qw(lxs l2m imp mrr cnv p2q tag sol); # internal workers +sub _drop_wq { + my ($self) = @_; + for my $wq (grep(defined, delete(@$self{@WQ_KEYS}))) { $wq->DESTROY } +} + # pronounced "exit": x_it(1 << 8) => exit(1); x_it(13) => SIGPIPE sub x_it ($$) { my ($self, $code) = @_; @@ -360,10 +365,7 @@ sub x_it ($$) { send($s, "x_it $code", MSG_EOR); } elsif ($self->{oneshot}) { # don't want to end up using $? from child processes - for my $f (@WQ_KEYS) { - my $wq = delete $self->{$f} or next; - $wq->DESTROY; - } + _drop_wq($self); # cleanup anything that has tempfiles or open file handles %PATH2CFG = (); delete @$self{qw(ovv dedupe sto cfg)}; @@ -392,11 +394,8 @@ sub qerr ($;@) { $_[0]->{opt}->{quiet} or err(shift, @_) } sub fail_handler ($;$$) { my ($lei, $code, $io) = @_; - for my $f (@WQ_KEYS) { - my $wq = delete $lei->{$f} or next; - $wq->wq_wait_old(undef, $lei) if $wq->wq_kill_old; # lei-daemon - } close($io) if $io; # needed to avoid warnings on SIGPIPE + _drop_wq($lei); x_it($lei, $code // (1 << 8)); } @@ -983,14 +982,7 @@ sub accept_dispatch { # Listener {post_accept} callback sub dclose { my ($self) = @_; delete $self->{-progress}; - for my $f (@WQ_KEYS) { - my $wq = delete $self->{$f} or next; - if ($wq->wq_kill) { - $wq->wq_close(0, undef, $self); - } elsif ($wq->wq_kill_old) { - $wq->wq_wait_old(undef, $self); - } - } + _drop_wq($self); close(delete $self->{1}) if $self->{1}; # may reap_compress $self->close if $self->{-event_init_done}; # PublicInbox::DS::close } diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm index 927fe550..48deca93 100644 --- a/lib/PublicInbox/LeiAuth.pm +++ b/lib/PublicInbox/LeiAuth.pm @@ -13,12 +13,15 @@ sub do_auth_atfork { # used by IPC WQ workers return if $wq->{-wq_worker_nr} != 0; my $lei = $wq->{lei}; my $net = $lei->{net}; - my $mics = $net->imap_common_init($lei); - my $nn = $net->nntp_common_init($lei); - pkt_do($lei->{pkt_op_p}, 'net_merge', $net) or - die "pkt_do net_merge: $!"; - $net->{mics_cached} = $mics if $mics; - $net->{nn_cached} = $nn if $nn; + eval { + my $mics = $net->imap_common_init($lei); + my $nn = $net->nntp_common_init($lei); + pkt_do($lei->{pkt_op_p}, 'net_merge', $net) or + die "pkt_do net_merge: $!"; + $net->{mics_cached} = $mics if $mics; + $net->{nn_cached} = $nn if $nn; + }; + $lei->fail($@) if $@; } sub net_merge_done1 { # bump merge-count in top-level lei-daemon diff --git a/lib/PublicInbox/NetReader.pm b/lib/PublicInbox/NetReader.pm index 6a52b479..c269d841 100644 --- a/lib/PublicInbox/NetReader.pm +++ b/lib/PublicInbox/NetReader.pm @@ -267,7 +267,7 @@ sub imap_common_init ($;$) { $mics->{$sec} //= mic_for($self, "$sec/", $mic_args, $lei); next unless $self->isa('PublicInbox::NetWriter'); my $dst = $uri->mailbox // next; - my $mic = $mics->{$sec}; + my $mic = $mics->{$sec} // die "Unable to continue\n"; next if $mic->exists($dst); # already exists $mic->create($dst) or die "CREATE $dst failed <$uri>: $@"; } -- cgit v1.2.3-24-ge0c7