about summary refs log tree commit homepage
diff options
authorEric Wong <e@80x24.org>2019-09-26 00:36:27 +0000
committerEric Wong <e@80x24.org>2019-09-26 00:36:27 +0000
commita6c9366f6d8ebb0a8716a198c4e40bb339aef766 (patch)
parent17bec61580fe1eab9b60b01d990af9046a1cb031 (diff)
httpd: disable Deflater middleware by default on Perl <5.18 p516-leak
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

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
1 files changed, 7 insertions, 1 deletions
diff --git a/script/public-inbox-httpd b/script/public-inbox-httpd
index b2464f4e..9b869f90 100755
--- a/script/public-inbox-httpd
+++ b/script/public-inbox-httpd
@@ -24,7 +24,13 @@ my $refresh = sub {
                 my $www = PublicInbox::WWW->new;
                 $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(