]> Sergey Matveev's repositories - public-inbox.git/commitdiff
git_async_cat: remove circular reference
authorEric Wong <e@yhbt.net>
Tue, 23 Jun 2020 23:21:12 +0000 (23:21 +0000)
committerEric Wong <e@yhbt.net>
Thu, 25 Jun 2020 05:37:59 +0000 (05:37 +0000)
While this circular reference was carefully managed to not leak
memory; it was still triggering a warning at -imapd/-nntpd
shutdown due to the EPOLL_CTL_DEL op failing after the $Epoll FD
gets closed.

So remove the circular reference by providing a ref to `undef',
instead.

lib/PublicInbox/Git.pm
lib/PublicInbox/GitAsyncCat.pm
lib/PublicInbox/IMAP.pm
lib/PublicInbox/NNTP.pm

index a55c48d5f523b5a9c0f43540abf0a24fa8545d04..776e483291b61fc639aa6b4aac5a4b66ce8c350a 100644 (file)
@@ -295,9 +295,7 @@ sub qx {
 sub cleanup {
        my ($self) = @_;
        local $in_cleanup = 1;
 sub cleanup {
        my ($self) = @_;
        local $in_cleanup = 1;
-       if (my $ac = $self->{async_cat}) {
-               $ac->close; # PublicInbox::GitAsyncCat::close -> EPOLL_CTL_DEL
-       }
+       delete $self->{async_cat};
        cat_async_wait($self);
        _destroy($self, qw(cat_rbuf in out pid));
        _destroy($self, qw(chk_rbuf in_c out_c pid_c err_c));
        cat_async_wait($self);
        _destroy($self, qw(cat_rbuf in out pid));
        _destroy($self, qw(chk_rbuf in_c out_c pid_c err_c));
index 8701e4cfe4889b654f509f18032a5ebd8413fc64..098101aed00843633eb239443a1a9c61f3cdf937 100644 (file)
@@ -15,20 +15,20 @@ use fields qw(git);
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 our @EXPORT = qw(git_async_cat);
 
 use PublicInbox::Syscall qw(EPOLLIN EPOLLET);
 our @EXPORT = qw(git_async_cat);
 
-sub new {
+sub _add {
        my ($class, $git) = @_;
        my $self = fields::new($class);
        $git->batch_prepare;
        $self->SUPER::new($git->{in}, EPOLLIN|EPOLLET);
        $self->{git} = $git;
        my ($class, $git) = @_;
        my $self = fields::new($class);
        $git->batch_prepare;
        $self->SUPER::new($git->{in}, EPOLLIN|EPOLLET);
        $self->{git} = $git;
-       $self;
+       \undef; # this is a true ref()
 }
 
 sub event_step {
        my ($self) = @_;
        my $git = $self->{git} or return; # ->close-ed
        my $inflight = $git->{inflight};
 }
 
 sub event_step {
        my ($self) = @_;
        my $git = $self->{git} or return; # ->close-ed
        my $inflight = $git->{inflight};
-       if (@$inflight) {
+       if ($inflight && @$inflight) {
                $git->cat_async_step($inflight);
                $self->requeue if @$inflight || exists $git->{cat_rbuf};
        }
                $git->cat_async_step($inflight);
                $self->requeue if @$inflight || exists $git->{cat_rbuf};
        }
@@ -37,7 +37,7 @@ sub event_step {
 sub close {
        my ($self) = @_;
        if (my $git = delete $self->{git}) {
 sub close {
        my ($self) = @_;
        if (my $git = delete $self->{git}) {
-               delete $git->{async_cat}; # drop circular reference
+               delete $git->{async_cat};
        }
        $self->SUPER::close; # PublicInbox::DS::close
 }
        }
        $self->SUPER::close; # PublicInbox::DS::close
 }
@@ -45,7 +45,7 @@ sub close {
 sub git_async_cat ($$$$) {
        my ($git, $oid, $cb, $arg) = @_;
        $git->cat_async($oid, $cb, $arg);
 sub git_async_cat ($$$$) {
        my ($git, $oid, $cb, $arg) = @_;
        $git->cat_async($oid, $cb, $arg);
-       $git->{async_cat} //= new(__PACKAGE__, $git); # circular reference
+       $git->{async_cat} //= _add(__PACKAGE__, $git);
 }
 
 1;
 }
 
 1;
index dec10d6142ba5c4ac82038d85b85cf6a32da65fb..0a6993c64c474526949bc3eac0590dc90dae18d8 100644 (file)
@@ -1294,7 +1294,7 @@ sub long_step {
        } elsif ($more) { # $self->{wbuf}:
                $self->update_idle_time;
 
        } elsif ($more) { # $self->{wbuf}:
                $self->update_idle_time;
 
-               # control passed to $more may be a GitAsyncCat object
+               # control passed to git_async_cat if $more == \undef
                requeue_once($self) if !ref($more);
        } else { # all done!
                delete $self->{long_cb};
                requeue_once($self) if !ref($more);
        } else { # all done!
                delete $self->{long_cb};
index 6df19f322b6e03aaf91ba76f61a524d047be7604..76f14bbd97d9bb943ad50bdddee4635c8de691f3 100644 (file)
@@ -553,8 +553,7 @@ sub cmd_article ($;$) {
        return $smsg unless ref $smsg;
        set_art($self, $art);
        $smsg->{nntp} = $self;
        return $smsg unless ref $smsg;
        set_art($self, $art);
        $smsg->{nntp} = $self;
-       git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg);
-       undef;
+       ${git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg)};
 }
 
 sub cmd_head ($;$) {
 }
 
 sub cmd_head ($;$) {
@@ -564,8 +563,7 @@ sub cmd_head ($;$) {
        set_art($self, $art);
        $smsg->{nntp} = $self;
        $smsg->{nntp_code} = 221;
        set_art($self, $art);
        $smsg->{nntp} = $self;
        $smsg->{nntp_code} = 221;
-       git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg);
-       undef;
+       ${git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg)};
 }
 
 sub cmd_body ($;$) {
 }
 
 sub cmd_body ($;$) {
@@ -575,8 +573,7 @@ sub cmd_body ($;$) {
        set_art($self, $art);
        $smsg->{nntp} = $self;
        $smsg->{nntp_code} = 222;
        set_art($self, $art);
        $smsg->{nntp} = $self;
        $smsg->{nntp_code} = 222;
-       git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg);
-       undef;
+       ${git_async_cat($self->{ng}->git, $smsg->{blob}, \&blob_cb, $smsg)};
 }
 
 sub cmd_stat ($;$) {
 }
 
 sub cmd_stat ($;$) {