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 CFB601F5AE; Mon, 6 Jul 2020 06:11:02 +0000 (UTC) Date: Mon, 6 Jul 2020 06:11:02 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH v2] git_async_cat: unref pipes on EOF from git->cleanup Message-ID: <20200706061102.GA15566@dcvr> References: <20200705225045.29291-1-e@yhbt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200705225045.29291-1-e@yhbt.net> List-Id: We avoided a managed circular reference in 10ee3548084c125f but introduced a pipe FD leak, instead. So handle the EOF we get when the "git cat-file --batch" process exits and closes its stdout FD. v2: remove ->close entirely. PublicInbox::Git->cleanup handles all cleanup. This prevents us from inadvertantly deleting the {async_cat} field associated with a different pipe than the one GAC is monitoring. Fixes: 10ee3548084c125f ("git_async_cat: remove circular reference") --- lib/PublicInbox/GitAsyncCat.pm | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm index 0b777204a..e79f416db 100644 --- a/lib/PublicInbox/GitAsyncCat.pm +++ b/lib/PublicInbox/GitAsyncCat.pm @@ -24,7 +24,8 @@ sub _add { sub event_step { my ($self) = @_; - my $git = $self->{git} or return; # ->close-ed + my $git = $self->{git}; + return $self->close if ($git->{in} // 0) != $self->{sock}; my $inflight = $git->{inflight}; if ($inflight && @$inflight) { $git->cat_async_step($inflight); @@ -32,14 +33,6 @@ sub event_step { } } -sub close { - my ($self) = @_; - if (my $git = delete $self->{git}) { - delete $git->{async_cat}; - } - $self->SUPER::close; # PublicInbox::DS::close -} - sub git_async_cat ($$$$) { my ($git, $oid, $cb, $arg) = @_; $git->cat_async($oid, $cb, $arg);