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-ASN: 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 EAF7C1F46C for ; Thu, 26 Sep 2019 01:50:38 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] httpd: disable Deflater middleware by default on Perl <5.18 Date: Thu, 26 Sep 2019 01:50:38 +0000 Message-Id: <20190926015038.29770-3-e@80x24.org> In-Reply-To: <20190926015038.29770-1-e@80x24.org> References: <20190926015038.29770-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Testing with perl-5.16.3-294.el7_6 RPM package on RHEL/CentOS 7, the Deflater middleware triggers a leak when used in conjunction with our push-based responses from PublicInbox::Qspawn. I could not find another solution to workaround the memory leak in this case, and I could not find a specific leak fix in the perl5180delta manpage[1] which looked like it would solve our problem. Attempting to workaround the issue proved futile. Using internal Deflater-specific keys to prevent deflating in GitHTTPBackend and Qspawn did not solve the problem: $env->{"plack.skip-deflater"} = 1; $env->{"psgix.no-compress"} = 1; Nor did forcing an invalid encoding via "git fetch": git -c http.extraheader=Accept-Encoding:gzap fetch So this appears to be a problem with Plack::Util::response_cb somewhere. This does NOT appear to be a problem with ref() leaking as in DS::next_tick[2], since I couldn't find where Plack::Middleware::Deflater or Plack::Util::response_cb would be calling ref() on a blessed reference to trigger a leak. Also, oddly enough, the ref() use for backwards compatibility at the top of PublicInbox::GitHTTPBackend::serve does NOT seem to trigger a leak on 5.16.3 due to [2]: # XXX compatibility... ugh, can we stop supporting this? $git = PublicInbox::Git->new($git) unless ref($git); [1] https://perldoc.perl.org/perl5180delta.html [2] https://rt.perl.org/Public/Bug/Display.html?id=114340 --- script/public-inbox-httpd | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/script/public-inbox-httpd b/script/public-inbox-httpd index b2464f4..9b869f9 100755 --- a/script/public-inbox-httpd +++ b/script/public-inbox-httpd @@ -24,7 +24,13 @@ my $refresh = sub { my $www = PublicInbox::WWW->new; $www->preload; $app = builder { - eval { + # Perl 5.16.3 leaks in our "push" response code path + # (e.g. Qspawn) due to something in + # Plack::Util::response_cb, regardless of whether the + # client is sending Accept-Encoding:gzip requests. + # perl5180delta documents many leak fixes, so assume + # 5.18+ is safe for now and bump the check as-need: + $] >= 5.018000 and eval { enable 'Deflater', content_type => [ qw( text/html -- EW