From 64aea34d06f71828b0bdd6ae177b9bcf22d752b4 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 24 May 2016 03:41:53 +0000 Subject: 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. --- lib/PublicInbox/GitHTTPBackend.pm | 38 ++++++++++--------------- lib/PublicInbox/Qspawn.pm | 52 +++++++++++++++++++++++++++++++++ t/qspawn.t | 60 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 23 deletions(-) create mode 100644 lib/PublicInbox/Qspawn.pm create mode 100644 t/qspawn.t diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm index ded56b33..9464cb49 100644 --- 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 index 00000000..9e4c8e08 --- /dev/null +++ b/lib/PublicInbox/Qspawn.pm @@ -0,0 +1,52 @@ +# Copyright (C) 2016 all contributors +# License: AGPL-3.0+ +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 index 00000000..05072e24 --- /dev/null +++ b/t/qspawn.t @@ -0,0 +1,60 @@ +# Copyright (C) 2016 all contributors +# License: AGPL-3.0+ +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; -- cgit v1.2.3-24-ge0c7