unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 07/21] lei: do_env combines fchdir and local
Date: Wed,  4 Oct 2023 03:49:19 +0000	[thread overview]
Message-ID: <20231004034933.3343930-8-e@80x24.org> (raw)
In-Reply-To: <20231004034933.3343930-1-e@80x24.org>

This will make switching $lei contexts less error-prone
and hopefully save us from some suprising bugs in the future.

Followup-to: 759885e60e59 (lei: ensure --stdin sets %ENV and $current_lei, 2023-09-14)
---
 lib/PublicInbox/LEI.pm        |  16 ++++--
 lib/PublicInbox/LeiAuth.pm    |   4 +-
 lib/PublicInbox/LeiConfig.pm  |  25 ++++----
 lib/PublicInbox/LeiInspect.pm |  28 ++++-----
 lib/PublicInbox/LeiLcat.pm    |  17 +++---
 lib/PublicInbox/LeiQuery.pm   |  19 +++----
 lib/PublicInbox/LeiXSearch.pm | 104 ++++++++++++++++------------------
 lib/PublicInbox/PktOp.pm      |  15 +++--
 8 files changed, 112 insertions(+), 116 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 8362800d..3408551b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -479,7 +479,6 @@ sub _drop_wq {
 # pronounced "exit": x_it(1 << 8) => exit(1); x_it(13) => SIGPIPE
 sub x_it ($$) {
 	my ($self, $code) = @_;
-	local $current_lei = $self;
 	# make sure client sees stdout before exit
 	$self->{1}->autoflush(1) if $self->{1};
 	stop_pager($self);
@@ -514,7 +513,6 @@ sub qfin { # show message on finalization (LeiFinmsg)
 
 sub fail_handler ($;$$) {
 	my ($lei, $code, $io) = @_;
-	local $current_lei = $lei;
 	close($io) if $io; # needed to avoid warnings on SIGPIPE
 	_drop_wq($lei);
 	x_it($lei, $code // (1 << 8));
@@ -785,11 +783,19 @@ sub lazy_cb ($$$) { # $pfx is _complete_ or lei_
 		$pkg->can($pfx.$ucmd) : undef;
 }
 
+sub do_env {
+	my $lei = shift;
+	fchdir($lei);
+	my $cb = shift // return ($lei, %{$lei->{env}}) ;
+	local ($current_lei, %ENV) = ($lei, %{$lei->{env}});
+	$cb = $lei->can($cb) if !ref($cb); # $cb may be a scalar sub name
+	eval { $cb->($lei, @_) };
+	$lei->fail($@) if $@;
+}
+
 sub dispatch {
 	my ($self, $cmd, @argv) = @_;
-	fchdir($self);
-	local %ENV = %{$self->{env}};
-	local $current_lei = $self; # for __WARN__
+	local ($current_lei, %ENV) = do_env($self);
 	$self->{2}->autoflush(1); # keep stdout buffered until x_it|DESTROY
 	return _help($self, 'no command given') unless defined($cmd);
 	# do not support Getopt bundling for this
diff --git a/lib/PublicInbox/LeiAuth.pm b/lib/PublicInbox/LeiAuth.pm
index 9b09cecf..76a4410d 100644
--- a/lib/PublicInbox/LeiAuth.pm
+++ b/lib/PublicInbox/LeiAuth.pm
@@ -57,7 +57,7 @@ sub net_merge_all { # called in wq worker via wq_broadcast
 # called by top-level lei-daemon when first worker is done with auth
 # passes updated net auth info to current workers
 sub net_merge_continue {
-	my ($wq, $lei, $net_new) = @_;
+	my ($lei, $wq, $net_new) = @_;
 	$wq->{-net_new} = $net_new; # for "lei up"
 	$wq->wq_broadcast('PublicInbox::LeiAuth::net_merge_all', $net_new);
 	$wq->net_merge_all_done($lei); # defined per-WQ
@@ -65,7 +65,7 @@ sub net_merge_continue {
 
 sub op_merge { # prepares PktOp->pair ops
 	my ($self, $ops, $wq, $lei) = @_;
-	$ops->{net_merge_continue} = [ \&net_merge_continue, $wq, $lei ];
+	$ops->{net_merge_continue} = [ \&net_merge_continue, $lei, $wq ];
 }
 
 sub new { bless \(my $x), __PACKAGE__ }
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index 76fc43e7..b3495487 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -16,24 +16,21 @@ sub cfg_do_edit ($;$) {
 	# run in script/lei foreground
 	my ($op_c, $op_p) = PublicInbox::PktOp->pair;
 	# $op_p will EOF when $EDITOR is done
-	$op_c->{ops} = { '' => [\&cfg_edit_done, $self] };
+	$op_c->{ops} = { '' => [\&cfg_edit_done, $lei, $self] };
 	$lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p->{op_p} ], $cmd, $env);
 }
 
-sub cfg_edit_done { # PktOp
-	my ($self) = @_;
-	eval {
-		open my $fh, '+>', undef or die "open($!)";
-		my $cfg = do {
-			local $self->{lei}->{2} = $fh;
-			$self->{lei}->cfg_dump($self->{-f});
-		} or do {
-			seek($fh, 0, SEEK_SET);
-			return cfg_do_edit($self, do { local $/; <$fh> });
-		};
-		$self->cfg_verify($cfg) if $self->can('cfg_verify');
+sub cfg_edit_done { # PktOp lei->do_env cb
+	my ($lei, $self) = @_;
+	open my $fh, '+>', undef or die "open($!)";
+	my $cfg = do {
+		local $lei->{2} = $fh;
+		$lei->cfg_dump($self->{-f});
+	} or do {
+		seek($fh, 0, SEEK_SET);
+		return cfg_do_edit($self, do { local $/; <$fh> });
 	};
-	$self->{lei}->fail($@) if $@;
+	$self->cfg_verify($cfg) if $self->can('cfg_verify');
 }
 
 sub lei_config {
diff --git a/lib/PublicInbox/LeiInspect.pm b/lib/PublicInbox/LeiInspect.pm
index 0455e739..f801610f 100644
--- a/lib/PublicInbox/LeiInspect.pm
+++ b/lib/PublicInbox/LeiInspect.pm
@@ -251,24 +251,20 @@ sub inspect_start ($$) {
 	$self->wq_close;
 }
 
+sub do_inspect { # lei->do_env cb
+	my ($lei) = @_;
+	my $str = delete $lei->{istr};
+	$str =~ s/\A[\r\n]*From [^\r\n]*\r?\n//s;
+	my $eml = PublicInbox::Eml->new(\$str);
+	inspect_start($lei, [ 'blob:'.$lei->git_oid($eml)->hexdigest,
+			map { "mid:$_" } @{mids($eml)} ]);
+}
+
 sub ins_add { # InputPipe->consume callback
 	my ($lei) = @_; # $_[1] = $rbuf
-	if (defined $_[1]) {
-		$_[1] eq '' and return eval {
-			$lei->fchdir;
-			local %ENV = %{$lei->{env}};
-			local $PublicInbox::LEI::current_lei = $lei;
-			my $str = delete $lei->{istr};
-			$str =~ s/\A[\r\n]*From [^\r\n]*\r?\n//s;
-			my $eml = PublicInbox::Eml->new(\$str);
-			inspect_start($lei, [
-				'blob:'.$lei->git_oid($eml)->hexdigest,
-				map { "mid:$_" } @{mids($eml)} ]);
-		};
-		$lei->{istr} .= $_[1];
-	} else {
-		$lei->fail("error reading stdin: $!");
-	}
+	$_[1] // return $lei->fail("error reading stdin: $!");
+	return $lei->{istr} .= $_[1] if $_[1] ne '';
+	$lei->do_env(\&do_inspect);
 }
 
 sub lei_inspect {
diff --git a/lib/PublicInbox/LeiLcat.pm b/lib/PublicInbox/LeiLcat.pm
index 7ed191c3..72875dc6 100644
--- a/lib/PublicInbox/LeiLcat.pm
+++ b/lib/PublicInbox/LeiLcat.pm
@@ -122,19 +122,18 @@ could not extract Message-ID from $x
 	@q ? join(' OR ', @q) : $lei->fail("no Message-ID in: @argv");
 }
 
+sub do_lcat { # lei->do_env cb
+	my ($lei) = @_;
+	my @argv = split(/\s+/, $lei->{mset_opt}->{qstr});
+	$lei->{mset_opt}->{qstr} = extract_all($lei, @argv) or return;
+	$lei->_start_query;
+}
+
 sub _stdin { # PublicInbox::InputPipe::consume callback for --stdin
 	my ($lei) = @_; # $_[1] = $rbuf
 	$_[1] // return $lei->fail("error reading stdin: $!");
 	return $lei->{mset_opt}->{qstr} .= $_[1] if $_[1] ne '';
-	eval {
-		$lei->fchdir;
-		local %ENV = %{$lei->{env}};
-		local $PublicInbox::LEI::current_lei = $lei;
-		my @argv = split(/\s+/, $lei->{mset_opt}->{qstr});
-		$lei->{mset_opt}->{qstr} = extract_all($lei, @argv) or return;
-		$lei->_start_query;
-	};
-	$lei->fail($@) if $@;
+	$lei->do_env(\&do_lcat);
 }
 
 sub lei_lcat {
diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm
index a23354f0..e2d8a096 100644
--- a/lib/PublicInbox/LeiQuery.pm
+++ b/lib/PublicInbox/LeiQuery.pm
@@ -59,20 +59,19 @@ sub _start_query { # used by "lei q" and "lei up"
 	$lxs->do_query($self);
 }
 
+sub do_qry { # do_env cb
+	my ($lei) = @_;
+	$lei->{mset_opt}->{q_raw} = $lei->{mset_opt}->{qstr};
+	$lei->{lse}->query_approxidate($lei->{lse}->git,
+					$lei->{mset_opt}->{qstr});
+	_start_query($lei);
+}
+
 sub qstr_add { # PublicInbox::InputPipe::consume callback for --stdin
 	my ($lei) = @_; # $_[1] = $rbuf
 	$_[1] // $lei->fail("error reading stdin: $!");
 	return $lei->{mset_opt}->{qstr} .= $_[1] if $_[1] ne '';
-	eval {
-		$lei->fchdir;
-		local %ENV = %{$lei->{env}};
-		local $PublicInbox::LEI::current_lei = $lei;
-		$lei->{mset_opt}->{q_raw} = $lei->{mset_opt}->{qstr};
-		$lei->{lse}->query_approxidate($lei->{lse}->git,
-						$lei->{mset_opt}->{qstr});
-		_start_query($lei);
-	};
-	$lei->fail($@) if $@;
+	$lei->do_env(\&do_qry);
 }
 
 # make the URI||PublicInbox::{Inbox,ExtSearch} a config-file friendly string
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 4e0849e8..8f63149e 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -405,62 +405,54 @@ sub xsearch_done_wait { # awaitpid cb
 
 sub query_done { # EOF callback for main daemon
 	my ($lei) = @_;
-	local $PublicInbox::LEI::current_lei = $lei;
-	eval {
-		my $l2m = delete $lei->{l2m};
-		delete $lei->{lxs};
-		($lei->{opt}->{'mail-sync'} && !$lei->{sto}) and
-			warn "BUG: {sto} missing with --mail-sync";
-		$lei->sto_done_request if $lei->{sto};
-		if (my $v2w = delete $lei->{v2w}) {
-			my $wait = $v2w->wq_do('done'); # may die
-			$v2w->wq_close;
-		}
-		$lei->{ovv}->ovv_end($lei);
-		if ($l2m) { # close() calls LeiToMail reap_compress
-			if (my $out = delete $lei->{old_1}) {
-				if (my $mbout = $lei->{1}) {
-					close($mbout) or die <<"";
+	my $l2m = delete $lei->{l2m};
+	delete $lei->{lxs};
+	($lei->{opt}->{'mail-sync'} && !$lei->{sto}) and
+		warn "BUG: {sto} missing with --mail-sync";
+	$lei->sto_done_request if $lei->{sto};
+	if (my $v2w = delete $lei->{v2w}) {
+		my $wait = $v2w->wq_do('done'); # may die
+		$v2w->wq_close;
+	}
+	$lei->{ovv}->ovv_end($lei);
+	if ($l2m) { # close() calls LeiToMail reap_compress
+		if (my $out = delete $lei->{old_1}) {
+			if (my $mbout = $lei->{1}) {
+				close($mbout) or die <<"";
 Error closing $lei->{ovv}->{dst}: \$!=$! \$?=$?
 
-				}
-				$lei->{1} = $out;
-			}
-			if ($l2m->lock_free) {
-				$l2m->poke_dst;
-				$lei->poke_mua;
-			} else { # mbox users
-				delete $l2m->{mbl}; # drop dotlock
 			}
+			$lei->{1} = $out;
 		}
-		if ($lei->{-progress}) {
-			my $tot = $lei->{-mset_total} // 0;
-			my $nr_w = $lei->{-nr_write} // 0;
-			my $d = ($lei->{-nr_seen} // 0) - $nr_w;
-			my $x = "$tot matches";
-			$x .= ", $d duplicates" if $d;
-			if ($l2m) {
-				my $m = "# $nr_w written to " .
-					"$lei->{ovv}->{dst} ($x)";
-				$nr_w ? $lei->qfin($m) : $lei->qerr($m);
-			} else {
-				$lei->qerr("# $x");
-			}
+		if ($l2m->lock_free) {
+			$l2m->poke_dst;
+			$lei->poke_mua;
+		} else { # mbox users
+			delete $l2m->{mbl}; # drop dotlock
 		}
-		$lei->start_mua if $l2m && !$l2m->lock_free;
-		$lei->dclose;
-	};
-	$lei->fail($@) if $@;
+	}
+	if ($lei->{-progress}) {
+		my $tot = $lei->{-mset_total} // 0;
+		my $nr_w = $lei->{-nr_write} // 0;
+		my $d = ($lei->{-nr_seen} // 0) - $nr_w;
+		my $x = "$tot matches";
+		$x .= ", $d duplicates" if $d;
+		if ($l2m) {
+			my $m = "# $nr_w written to " .
+				"$lei->{ovv}->{dst} ($x)";
+			$nr_w ? $lei->qfin($m) : $lei->qerr($m);
+		} else {
+			$lei->qerr("# $x");
+		}
+	}
+	$lei->start_mua if $l2m && !$l2m->lock_free;
+	$lei->dclose;
 }
 
 sub do_post_augment {
 	my ($lei) = @_;
-	local $PublicInbox::LEI::current_lei = $lei;
 	my $l2m = $lei->{l2m} or return; # client disconnected
-	eval {
-		$lei->fchdir;
-		$l2m->post_augment($lei);
-	};
+	eval { $l2m->post_augment($lei) };
 	my $err = $@;
 	if ($err) {
 		if (my $lxs = delete $lei->{lxs}) {
@@ -518,7 +510,7 @@ sub start_query ($$) { # always runs in main (lei-daemon) process
 }
 
 sub incr_start_query { # called whenever an l2m shard starts do_post_auth
-	my ($self, $lei) = @_;
+	my ($lei, $self) = @_;
 	my $l2m = $lei->{l2m};
 	return if ++$self->{nr_start_query} != $l2m->{-wq_nr_workers};
 	start_query($self, $lei);
@@ -534,16 +526,16 @@ sub do_query {
 	my ($self, $lei) = @_;
 	my $l2m = $lei->{l2m};
 	my $ops = {
-		'sigpipe_handler' => [ $lei ],
-		'fail_handler' => [ $lei ],
-		'do_post_augment' => [ \&do_post_augment, $lei ],
-		'incr_post_augment' => [ \&incr_post_augment, $lei ],
+		sigpipe_handler => [ $lei ],
+		fail_handler => [ $lei ],
+		do_post_augment => [ \&do_post_augment, $lei ],
+		incr_post_augment => [ \&incr_post_augment, $lei ],
 		'' => [ \&query_done, $lei ],
-		'mset_progress' => [ \&mset_progress, $lei ],
-		'l2m_progress' => [ \&l2m_progress, $lei ],
-		'x_it' => [ $lei ],
-		'child_error' => [ $lei ],
-		'incr_start_query' => [ $self, $lei ],
+		mset_progress => [ \&mset_progress, $lei ],
+		l2m_progress => [ \&l2m_progress, $lei ],
+		x_it => [ $lei ],
+		child_error => [ $lei ],
+		incr_start_query => [ \&incr_start_query, $lei, $self ],
 	};
 	$lei->{auth}->op_merge($ops, $l2m, $lei) if $l2m && $lei->{auth};
 	my $end = $lei->pkt_op_pair;
diff --git a/lib/PublicInbox/PktOp.pm b/lib/PublicInbox/PktOp.pm
index dc432307..1bcdd799 100644
--- a/lib/PublicInbox/PktOp.pm
+++ b/lib/PublicInbox/PktOp.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
 # op dispatch socket, reads a message, runs a sub
@@ -6,8 +6,7 @@
 # Used for lei_xsearch and maybe other things
 # "command" => [ $sub, @fixed_operands ]
 package PublicInbox::PktOp;
-use strict;
-use v5.10.1;
+use v5.12;
 use parent qw(PublicInbox::DS);
 use Errno qw(EAGAIN ECONNRESET);
 use PublicInbox::Syscall qw(EPOLLIN);
@@ -55,7 +54,15 @@ sub event_step {
 	my $op = $self->{ops}->{$cmd //= $msg};
 	if ($op) {
 		my ($obj, @args) = (@$op, @pargs);
-		blessed($obj) ? $obj->$cmd(@args) : $obj->(@args);
+		if (blessed($args[0]) && $args[0]->can('do_env')) {
+			my $lei = shift @args;
+			$lei->do_env($obj, @args);
+		} elsif (blessed($obj)) {
+			$obj->can('do_env') ? $obj->do_env($cmd, @args)
+						: $obj->$cmd(@args);
+		} else {
+			$obj->(@args);
+		}
 	} elsif ($msg ne '') {
 		die "BUG: unknown message: `$cmd'";
 	}

  parent reply	other threads:[~2023-10-04  3:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-04  3:49 [PATCH 00/21] lei + IPC related stuff Eric Wong
2023-10-04  3:49 ` [PATCH 01/21] lei: drop stores explicitly at daemon shutdown Eric Wong
2023-10-04  3:49 ` [PATCH 02/21] ds: hoist out close_non_busy Eric Wong
2023-10-04  3:49 ` [PATCH 03/21] ds: don't pass FD map to post_loop_do callback Eric Wong
2023-10-04  3:49 ` [PATCH 04/21] move all non-test @post_loop_do into named subs Eric Wong
2023-10-04  3:49 ` [PATCH 05/21] lei: close DirIdle (inotify) early at daemon shutdown Eric Wong
2023-10-04  3:49 ` [PATCH 06/21] input_pipe: {args} is never undefined Eric Wong
2023-10-04  3:49 ` Eric Wong [this message]
2023-10-04  3:49 ` [PATCH 08/21] lei: get rid of l2m_progress PktOp callback Eric Wong
2023-10-04  3:49 ` [PATCH 09/21] t/lei_to_mail: modernize and document test Eric Wong
2023-10-04  3:49 ` [PATCH 10/21] lei: reuse PublicInbox::Config::noop Eric Wong
2023-10-04  3:49 ` [PATCH 11/21] lei: keep signals blocked on daemon shutdown Eric Wong
2023-10-04  3:49 ` [PATCH 12/21] mbox_lock: retry on EINTR and use autodie Eric Wong
2023-10-04  3:49 ` [PATCH 13/21] lock: retry on EINTR, improve error reporting Eric Wong
2023-10-04  3:49 ` [PATCH 14/21] treewide: use PublicInbox::Lock->new Eric Wong
2023-10-04  3:49 ` [PATCH 15/21] gcf2: use PublicInbox::Lock Eric Wong
2023-10-04  3:49 ` [PATCH 16/21] spawn: use autodie and PublicInbox::Lock Eric Wong
2023-10-04  3:49 ` [PATCH 17/21] xap_helper: retry flock on EINTR Eric Wong
2023-10-04  3:49 ` [PATCH 18/21] xap_helper.pm: use EINTR-aware recv_cmd Eric Wong
2023-10-04  3:49 ` [PATCH 19/21] spawn: drop checks for directory writability Eric Wong
2023-10-04  3:49 ` [PATCH 20/21] lei: document and local-ize $OPT hashref Eric Wong
2023-10-04  3:49 ` [PATCH 21/21] searchidx: fix redundant `in' in warning message Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231004034933.3343930-8-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).