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-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 538C41FB0E for ; Tue, 23 Mar 2021 11:48:09 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 5/5] lei: improve management around short-lived workers Date: Tue, 23 Mar 2021 11:48:08 +0000 Message-Id: <20210323114808.7605-6-e@80x24.org> In-Reply-To: <20210323114808.7605-1-e@80x24.org> References: <20210323114808.7605-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Instead of creating a short-lived circular reference, ensure they don't exist in the first place. Note the following changes to hold an extra ref to $sto: - $self->_lei_store(1)->write_prepare($self); + my $sto = $self->_lei_store(1); + $sto->write_prepare($self); I'm not a perlguts expert, but I actually wanted to switch to the one-line version for LeiImport, but xt/lei-auth-fail.t was getting stuck for some reason. It seems the extra ref to the LeiStore ($sto) object is necessary. --- lib/PublicInbox/LEI.pm | 1 - lib/PublicInbox/LeiConvert.pm | 3 ++- lib/PublicInbox/LeiExternal.pm | 3 ++- lib/PublicInbox/LeiImport.pm | 20 +++++++------------- lib/PublicInbox/LeiMark.pm | 2 +- lib/PublicInbox/LeiMirror.pm | 2 +- lib/PublicInbox/LeiP2q.pm | 5 +++-- lib/PublicInbox/LeiQuery.pm | 2 +- 8 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index d3ac19b2..8cbaac01 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -462,7 +462,6 @@ sub _lei_atfork_child { open STDERR, '+>&='.fileno($self->{2}) or warn "open $!"; delete $self->{0}; } - delete @$self{qw(cnv mark imp)}; for (delete @$self{qw(3 old_1 au_done)}) { close($_) if defined($_); } diff --git a/lib/PublicInbox/LeiConvert.pm b/lib/PublicInbox/LeiConvert.pm index bc86fe25..0cc65108 100644 --- a/lib/PublicInbox/LeiConvert.pm +++ b/lib/PublicInbox/LeiConvert.pm @@ -46,13 +46,14 @@ sub lei_convert { # the main "lei convert" method my ($lei, @inputs) = @_; $lei->{opt}->{kw} //= 1; $lei->{opt}->{dedupe} //= 'none'; - my $self = $lei->{cnv} = bless {}, __PACKAGE__; + my $self = bless {}, __PACKAGE__; my $ovv = PublicInbox::LeiOverview->new($lei, 'out-format'); $lei->{l2m} or return $lei->fail("output not specified or is not a mail destination"); $lei->{opt}->{augment} = 1 unless $ovv->{dst} eq '/dev/stdout'; $self->prepare_inputs($lei, \@inputs) or return; my $op = $lei->workers_start($self, 'lei_convert', 1); + $lei->{cnv} = $self; $self->wq_io_do('do_convert', []); $self->wq_close(1); while ($op && $op->{sock}) { $op->event_step } diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm index 9a555831..56d6ef39 100644 --- a/lib/PublicInbox/LeiExternal.pm +++ b/lib/PublicInbox/LeiExternal.pm @@ -144,7 +144,8 @@ sub add_external_finish { sub lei_add_external { my ($self, $location) = @_; - $self->_lei_store(1)->write_prepare($self); + my $sto = $self->_lei_store(1); + $sto->write_prepare($self); my $opt = $self->{opt}; my $mirror = $opt->{mirror} // do { my @fail; diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm index 991c84f2..9da6b7f9 100644 --- a/lib/PublicInbox/LeiImport.pm +++ b/lib/PublicInbox/LeiImport.pm @@ -58,9 +58,13 @@ sub net_merge_complete { # callback used by LeiAuth $self->wq_close(1); } -sub import_start { - my ($lei) = @_; - my $self = $lei->{imp}; +sub lei_import { # the main "lei import" method + my ($lei, @inputs) = @_; + my $sto = $lei->_lei_store(1); + $sto->write_prepare($lei); + my $self = bless {}, __PACKAGE__; + $self->{-import_kw} = $lei->{opt}->{kw} // 1; + $self->prepare_inputs($lei, \@inputs) or return; $lei->ale; # initialize for workers to read my $j = $lei->{opt}->{jobs} // scalar(@{$self->{inputs}}) || 1; if (my $net = $lei->{net}) { @@ -79,16 +83,6 @@ sub import_start { while ($op && $op->{sock}) { $op->event_step } } -sub lei_import { # the main "lei import" method - my ($lei, @inputs) = @_; - my $sto = $lei->_lei_store(1); - $sto->write_prepare($lei); - my $self = $lei->{imp} = bless {}, __PACKAGE__; - $self->{-import_kw} = $lei->{opt}->{kw} // 1; - $self->prepare_inputs($lei, \@inputs) or return; - import_start($lei); -} - no warnings 'once'; *ipc_atfork_child = \&PublicInbox::LeiInput::input_only_atfork_child; diff --git a/lib/PublicInbox/LeiMark.pm b/lib/PublicInbox/LeiMark.pm index 3b5e6c2c..9d77f4b4 100644 --- a/lib/PublicInbox/LeiMark.pm +++ b/lib/PublicInbox/LeiMark.pm @@ -105,8 +105,8 @@ sub input_net_cb { # imap_each, nntp_each cb sub lei_mark { # the "lei mark" method my ($lei, @argv) = @_; my $sto = $lei->_lei_store(1); - my $self = $lei->{mark} = bless { missing => 0 }, __PACKAGE__; $sto->write_prepare($lei); + my $self = bless { missing => 0 }, __PACKAGE__; $lei->ale; # refresh and prepare my $vmd_mod = vmd_mod_extract(\@argv); return $lei->fail(join("\n", @{$vmd_mod->{err}})) if $vmd_mod->{err}; diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm index c916f2d0..6e62625d 100644 --- a/lib/PublicInbox/LeiMirror.pm +++ b/lib/PublicInbox/LeiMirror.pm @@ -269,7 +269,6 @@ sub do_mirror { # via wq_io_do sub start { my ($cls, $lei, $src, $dst) = @_; my $self = bless { lei => $lei, src => $src, dst => $dst }, $cls; - $lei->{mrr} = $self; if ($src =~ m!https?://!) { require URI; require PublicInbox::LeiCurl; @@ -281,6 +280,7 @@ sub start { my $op = $lei->workers_start($self, 'lei_mirror', 1, { '' => [ \&mirror_done, $lei ] }); + $lei->{mrr} = $self; $self->wq_io_do('do_mirror', []); $self->wq_close(1); while ($op && $op->{sock}) { $op->event_step } diff --git a/lib/PublicInbox/LeiP2q.pm b/lib/PublicInbox/LeiP2q.pm index 0f7ffb5f..fda055fe 100644 --- a/lib/PublicInbox/LeiP2q.pm +++ b/lib/PublicInbox/LeiP2q.pm @@ -176,13 +176,14 @@ sub do_p2q { # via wq_do sub lei_p2q { # the "lei patch-to-query" entry point my ($lei, $input) = @_; - my $self = $lei->{p2q} = bless {}, __PACKAGE__; + my $self = bless {}, __PACKAGE__; if ($lei->{opt}->{stdin}) { $self->{0} = delete $lei->{0}; # guard from _lei_atfork_child } else { $self->{input} = $input; } - my $op = $lei->workers_start($self, 'lei patch2query', 1); + my $op = $lei->workers_start($self, 'lei_p2q', 1); + $lei->{p2q} = $self; $self->wq_io_do('do_p2q', []); $self->wq_close(1); while ($op && $op->{sock}) { $op->event_step } diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm index 148e8524..84996e7e 100644 --- a/lib/PublicInbox/LeiQuery.pm +++ b/lib/PublicInbox/LeiQuery.pm @@ -50,11 +50,11 @@ sub lei_q { # --local is enabled by default unless --only is used # we'll allow "--only $LOCATION --local" my $sto = $self->_lei_store(1); - my $lse = $sto->search; if (($opt->{'import-remote'} //= 1) | (($opt->{'import-before'} //= \1) ? 1 : 0)) { $sto->write_prepare($self); } + my $lse = $sto->search; if ($opt->{'local'} //= scalar(@only) ? 0 : 1) { $lxs->prepare_external($lse); }