git-http-backend: use qspawn to limit running processes

Having an excessive amount of git-pack-objects processes is
dangerous to the health of the server.  Queue up process spawning
for long-running responses and serve them sequentially, instead.

diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm
index ded56b3330f742c95d965ab59ef18fd74b561215..9464cb499a3f0c4744b8b33eec1559e415da4887 100644 (file)
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -8,9 +8,9 @@ use strict;
 use warnings;
 use Fcntl qw(:seek);
 use IO::File;
-use PublicInbox::Spawn qw(spawn);
 use HTTP::Date qw(time2str);
 use HTTP::Status qw(status_message);
+use PublicInbox::Qspawn;
 
 # n.b. serving "description" and "cloneurl" should be innocuous enough to
 # not cause problems.  serving "config" might...
@@ -167,11 +167,6 @@ sub serve_smart {
        unless (defined $fd && $fd >= 0) {
                $in = input_to_file($env) or return r(500);
        }
-       my ($rpipe, $wpipe);
-       unless (pipe($rpipe, $wpipe)) {
-               err($env, "error creating pipe: $! - going static");
-               return;
-       }
        my %env = %ENV;
        # GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
        # may be set in the server-process and are passed as-is
@@ -187,20 +182,13 @@ sub serve_smart {
        my $git_dir = $git->{git_dir};
        $env{GIT_HTTP_EXPORT_ALL} = '1';
        $env{PATH_TRANSLATED} = "$git_dir/$path";
-       my %rdr = ( 0 => fileno($in), 1 => fileno($wpipe) );
-       my $pid = spawn([qw(git http-backend)], \%env, \%rdr);
-       unless (defined $pid) {
-               err($env, "error spawning: $! - going static");
-               return;
-       }
-       $wpipe = $in = undef;
-       my $fh;
+       my %rdr = ( 0 => fileno($in) );
+       my $x = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, \%rdr);
+       my ($fh, $rpipe);
        my $end = sub {
                $rpipe = undef;
-               my $e = $pid == waitpid($pid, 0) ?
-                       $? : "PID:$pid still running?";
-               if ($e) {
-                       err($env, "git http-backend ($git_dir): $e");
+               if (my $err = $x->finish) {
+                       err($env, "git http-backend ($git_dir): $err");
                        drop_client($env);
                }
                $fh->close if $fh; # async-only
@@ -248,11 +236,15 @@ sub serve_smart {
                # holding the input here is a waste of FDs and memory
                $env->{'psgi.input'} = undef;
 
-               if ($async) {
-                       $async = $async->($rpipe, $cb, $end);
-               } else { # generic PSGI
-                       $cb->() while $rd_hdr;
-               }
+               $x->start(sub { # may run later, much later...
+                       ($rpipe) = @_;
+                       $in = undef;
+                       if ($async) {
+                               $async = $async->($rpipe, $cb, $end);
+                       } else { # generic PSGI
+                               $cb->() while $rd_hdr;
+                       }
+               });
        };
 }
 

diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm
new file mode 100644 (file)
index 0000000..9e4c8e0
--- /dev/null
+++ b/lib/PublicInbox/Qspawn.pm
@@ -0,0 +1,52 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::Qspawn;
+use strict;
+use warnings;
+use PublicInbox::Spawn qw(popen_rd);
+our $LIMIT = 1;
+my $running = 0;
+my @run_queue;
+
+sub new ($$$;) {
+       my ($class, $cmd, $env, $opt) = @_;
+       bless { args => [ $cmd, $env, $opt ] }, $class;
+}
+
+sub _do_spawn {
+       my ($self, $cb) = @_;
+       my $err;
+       ($self->{rpipe}, $self->{pid}) = popen_rd(@{$self->{args}});
+       if ($self->{pid}) {
+               $running++;
+       } else {
+               $self->{err} = $!;
+       }
+       $cb->($self->{rpipe});
+}
+
+sub finish ($) {
+       my ($self) = @_;
+       if (delete $self->{rpipe}) {
+               my $pid = delete $self->{pid};
+               $self->{err} = $pid == waitpid($pid, 0) ? $? :
+                               "PID:$pid still running?";
+               $running--;
+       }
+       if (my $next = shift @run_queue) {
+               _do_spawn(@$next);
+       }
+       $self->{err};
+}
+
+sub start ($$) {
+       my ($self, $cb) = @_;
+
+       if ($running < $LIMIT) {
+               _do_spawn($self, $cb);
+       } else {
+               push @run_queue, [ $self, $cb ];
+       }
+}
+
+1;

diff --git a/t/qspawn.t b/t/qspawn.t
new file mode 100644 (file)
index 0000000..05072e2
--- /dev/null
+++ b/t/qspawn.t
@@ -0,0 +1,60 @@
+# Copyright (C) 2016 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use Test::More;
+use_ok 'PublicInbox::Qspawn';
+{
+       my $x = PublicInbox::Qspawn->new([qw(true)]);
+       my $run = 0;
+       $x->start(sub {
+               my ($rpipe) = @_;
+               is(0, sysread($rpipe, my $buf, 1), 'read zero bytes');
+               ok(!$x->finish, 'no error on finish');
+               $run = 1;
+       });
+       is($run, 1, 'callback ran alright');
+}
+
+{
+       my $x = PublicInbox::Qspawn->new([qw(false)]);
+       my $run = 0;
+       $x->start(sub {
+               my ($rpipe) = @_;
+               is(0, sysread($rpipe, my $buf, 1), 'read zero bytes from false');
+               my $err = $x->finish;
+               is($err, 256, 'error on finish');
+               $run = 1;
+       });
+       is($run, 1, 'callback ran alright');
+}
+
+foreach my $cmd ([qw(sleep 1)], [qw(sh -c), 'sleep 1; false']) {
+       my $s = PublicInbox::Qspawn->new($cmd);
+       my @run;
+       $s->start(sub {
+               my ($rpipe) = @_;
+               push @run, 'sleep';
+               is(0, sysread($rpipe, my $buf, 1), 'read zero bytes');
+       });
+       my $n = 0;
+       my @t = map {
+               my $i = $n++;
+               my $x = PublicInbox::Qspawn->new([qw(true)]);
+               $x->start(sub {
+                       my ($rpipe) = @_;
+                       push @run, $i;
+               });
+               [$x, $i]
+       } (0..2);
+
+       if ($cmd->[-1] =~ /false\z/) {
+               ok($s->finish, 'got error on false after sleep');
+       } else {
+               ok(!$s->finish, 'no error on sleep');
+       }
+       ok(!$_->[0]->finish, "true $_->[1] succeeded") foreach @t;
+       is_deeply([qw(sleep 0 1 2)], \@run, 'ran in order');
+}
+
+done_testing();
+
+1;