From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id D342E1F9E5 for ; Sat, 3 Apr 2021 02:24:27 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/6] lei q: ensure wq workers shutdown on IMAP auth failures Date: Sat, 3 Apr 2021 02:24:23 +0000 Message-Id: <20210403022427.2430-3-e@80x24.org> In-Reply-To: <20210403022427.2430-1-e@80x24.org> References: <20210403022427.2430-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: 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>: $@"; }