user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 2/3] git: async_abort includes --batch-check requests
Date: Fri,  8 Oct 2021 10:20:03 +0000	[thread overview]
Message-ID: <20211008102004.7879-3-e@80x24.org> (raw)
In-Reply-To: <20211008102004.7879-1-e@80x24.org>

We need to abort both check-only and cat requests when
aborting, since we'll be aborting more aggressively
in in read-write paths.
---
 lib/PublicInbox/Gcf2Client.pm  |  4 ++--
 lib/PublicInbox/Git.pm         | 40 +++++++++++++++++++---------------
 lib/PublicInbox/GitAsyncCat.pm |  4 ++--
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/lib/PublicInbox/Gcf2Client.pm b/lib/PublicInbox/Gcf2Client.pm
index c5695db140cd..09c3aa06f6be 100644
--- a/lib/PublicInbox/Gcf2Client.pm
+++ b/lib/PublicInbox/Gcf2Client.pm
@@ -18,7 +18,7 @@ use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 #	pid.owner => process which spawned {pid}
 #	in => same as {sock}, for compatibility with PublicInbox::Git
 #	inflight => array (see PublicInbox::Git)
-#	cat_rbuf => scalarref, may be non-existent or empty
+#	rbuf => scalarref, may be non-existent or empty
 sub new  {
 	my ($rdr) = @_;
 	my $self = bless {}, __PACKAGE__;
@@ -68,7 +68,7 @@ sub event_step {
 		return $self->close unless $self->{in}; # process died
 
 		# ok, more to do, requeue for fairness
-		$self->requeue if @$inflight || exists($self->{cat_rbuf});
+		$self->requeue if @$inflight || exists($self->{rbuf});
 	}
 }
 
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 77783000573f..f15ace1a3d62 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -202,7 +202,7 @@ sub cat_async_step ($$) {
 	my ($self, $inflight) = @_;
 	die 'BUG: inflight empty or odd' if scalar(@$inflight) < 3;
 	my ($req, $cb, $arg) = splice(@$inflight, 0, 3);
-	my $rbuf = delete($self->{cat_rbuf}) // \(my $new = '');
+	my $rbuf = delete($self->{rbuf}) // \(my $new = '');
 	my ($bref, $oid, $type, $size);
 	my $head = my_readline($self->{in}, $rbuf);
 	# ->fail may be called via Gcf2Client.pm
@@ -225,7 +225,7 @@ sub cat_async_step ($$) {
 		my $err = $! ? " ($!)" : '';
 		$self->fail("bad result from async cat-file: $head$err");
 	}
-	$self->{cat_rbuf} = $rbuf if $$rbuf ne '';
+	$self->{rbuf} = $rbuf if $$rbuf ne '';
 	eval { $cb->($bref, $oid, $type, $size, $arg) };
 	warn "E: $oid: $@\n" if $@;
 }
@@ -259,7 +259,7 @@ sub check_async_step ($$) {
 	my ($self, $inflight_c) = @_;
 	die 'BUG: inflight empty or odd' if scalar(@$inflight_c) < 3;
 	my ($req, $cb, $arg) = splice(@$inflight_c, 0, 3);
-	my $rbuf = delete($self->{chk_rbuf}) // \(my $new = '');
+	my $rbuf = delete($self->{rbuf_c}) // \(my $new = '');
 	chomp(my $line = my_readline($self->{in_c}, $rbuf));
 	my ($hex, $type, $size) = split(/ /, $line);
 
@@ -271,7 +271,7 @@ sub check_async_step ($$) {
 		my $ret = my_read($self->{in_c}, $rbuf, $type + 1);
 		$self->fail(defined($ret) ? 'read EOF' : "read: $!") if !$ret;
 	}
-	$self->{chk_rbuf} = $rbuf if $$rbuf ne '';
+	$self->{rbuf_c} = $rbuf if $$rbuf ne '';
 	eval { $cb->($hex, $type, $size, $arg, $self) };
 	warn "E: check($req) $@\n" if $@;
 }
@@ -333,24 +333,28 @@ sub _destroy {
 	dwaitpid($p) if $$ == $self->{"$pid.owner"};
 }
 
-sub cat_async_abort ($) {
+sub async_abort ($) {
 	my ($self) = @_;
-	if (my $inflight = $self->{inflight}) {
-		while (@$inflight) {
-			my ($req, $cb, $arg) = splice(@$inflight, 0, 3);
-			$req =~ s/ .*//; # drop git_dir for Gcf2Client
-			eval { $cb->(undef, $req, undef, undef, $arg) };
-			warn "E: $req: $@ (in abort)\n" if $@;
+	while (scalar(@{$self->{inflight_c} // []}) ||
+			scalar(@{$self->{inflight} // []})) {
+		for my $c ('', '_c') {
+			my $q = $self->{"inflight$c"};
+			while (@$q) {
+				my ($req, $cb, $arg) = splice(@$q, 0, 3);
+				$req =~ s/ .*//; # drop git_dir for Gcf2Client
+				eval { $cb->(undef, $req, undef, undef, $arg) };
+				warn "E: $req: $@ (in abort)\n" if $@;
+			}
+			delete $self->{"inflight$c"};
+			delete $self->{"rbuf$c"};
 		}
-		delete $self->{cat_rbuf};
-		delete $self->{inflight};
 	}
 	cleanup($self);
 }
 
 sub fail { # may be augmented in subclasses
 	my ($self, $msg) = @_;
-	cat_async_abort($self);
+	async_abort($self);
 	croak(ref($self) . ' ' . ($self->{git_dir} // '') . ": $msg");
 }
 
@@ -391,8 +395,8 @@ sub async_wait_all ($) {
 	my ($self) = @_;
 	while (scalar(@{$self->{inflight_c} // []}) ||
 			scalar(@{$self->{inflight} // []})) {
-		$self->check_async_wait;
-		$self->cat_async_wait;
+		check_async_wait($self);
+		cat_async_wait($self);
 	}
 }
 
@@ -406,8 +410,8 @@ sub cleanup {
 	async_wait_all($self);
 	delete $self->{inflight};
 	delete $self->{inflight_c};
-	_destroy($self, qw(cat_rbuf in out pid));
-	_destroy($self, qw(chk_rbuf in_c out_c pid_c err_c));
+	_destroy($self, qw(rbuf in out pid));
+	_destroy($self, qw(rbuf_c in_c out_c pid_c err_c));
 	undef;
 }
 
diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm
index 57c194d9e45f..cea3f539234a 100644
--- a/lib/PublicInbox/GitAsyncCat.pm
+++ b/lib/PublicInbox/GitAsyncCat.pm
@@ -16,7 +16,7 @@ our $GCF2C; # singleton PublicInbox::Gcf2Client
 sub close {
 	my ($self) = @_;
 	if (my $git = delete $self->{git}) {
-		$git->cat_async_abort;
+		$git->async_abort;
 	}
 	$self->SUPER::close; # PublicInbox::DS::close
 }
@@ -32,7 +32,7 @@ sub event_step {
 		# child death?
 		if (($git->{in} // 0) != ($self->{sock} // 1)) {
 			$self->close;
-		} elsif (@$inflight || exists $git->{cat_rbuf}) {
+		} elsif (@$inflight || exists $git->{rbuf}) {
 			# ok, more to do, requeue for fairness
 			$self->requeue;
 		}

  parent reply	other threads:[~2021-10-08 10:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 10:20 [PATCH 0/3] git: propagate die from async callbacks Eric Wong
2021-10-08 10:20 ` [PATCH 1/3] git: use async_wait_all everywhere Eric Wong
2021-10-08 10:20 ` Eric Wong [this message]
2021-10-08 10:20 ` [PATCH 3/3] git: fatalize async callback errors by default 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=20211008102004.7879-3-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    --subject='Re: [PATCH 2/3] git: async_abort includes --batch-check requests' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/public-inbox.git

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).