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 168EF1FA1A for ; Thu, 21 Jan 2021 19:46:25 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 06/12] lei_to_mail: avoid segfault on exit Date: Thu, 21 Jan 2021 19:46:18 +0000 Message-Id: <20210121194624.32002-7-e@80x24.org> In-Reply-To: <20210121194624.32002-1-e@80x24.org> References: <20210121194624.32002-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Worker exit causes DESTROY ordering to become unpredictable and leads to Perl segfaulting. Instead, rely on OnDestroy and explicit triggering after wq_worker_loop to ensure we finish all outstanding git requests before worker exit. --- lib/PublicInbox/LeiToMail.pm | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm index 87cc9c47..cea68319 100644 --- a/lib/PublicInbox/LeiToMail.pm +++ b/lib/PublicInbox/LeiToMail.pm @@ -11,6 +11,7 @@ use PublicInbox::Lock; use PublicInbox::ProcessPipe; use PublicInbox::Spawn qw(which spawn popen_rd); use PublicInbox::LeiDedupe; +use PublicInbox::OnDestroy; use Symbol qw(gensym); use IO::Handle; # ->autoflush use Fcntl qw(SEEK_SET SEEK_END O_CREAT O_EXCL O_WRONLY); @@ -472,12 +473,21 @@ sub ipc_atfork_prepare { $self->SUPER::ipc_atfork_prepare; # PublicInbox::IPC } -sub DESTROY { +# We rely on OnDestroy to run this before ->DESTROY, since ->DESTROY +# ordering is unstable at worker exit and may cause segfaults +sub reap_gits { my ($self) = @_; - for my $pid_git (grep(/\A$$\0/, keys %$self)) { - $self->{$pid_git}->async_wait_all; + for my $git (delete @$self{grep(/\A$$\0/, keys %$self)}) { + $git->async_wait_all; } - $self->SUPER::DESTROY; # PublicInbox::IPC +} + +sub ipc_atfork_child { # runs after IPC::wq_worker_loop + my ($self) = @_; + $self->SUPER::ipc_atfork_child; + # reap_gits needs to run before $self->DESTROY, + # IPC.pm will ensure that. + PublicInbox::OnDestroy->new($$, \&reap_gits, $self); } 1;