]> Sergey Matveev's repositories - public-inbox.git/blobdiff - lib/PublicInbox/GitHTTPBackend.pm
http: improve error handling for aborted responses
[public-inbox.git] / lib / PublicInbox / GitHTTPBackend.pm
index 8e6d8b648985445e93d215b5c8faf0525f2fb41a..4b3969346acefda27f49ffd47c5f0b26db49a1c6 100644 (file)
@@ -7,8 +7,17 @@ package PublicInbox::GitHTTPBackend;
 use strict;
 use warnings;
 use Fcntl qw(:seek);
+use IO::File;
 use PublicInbox::Spawn qw(spawn);
 
+# TODO: make configurable, but keep in mind it's better to have
+# multiple -httpd worker processes which are already scaled to
+# the proper number of CPUs and memory.  git-pack-objects(1) may
+# also use threads and bust memory limits, too, so I recommend
+# limiting threads to 1 (via `pack.threads` knob in git) for serving.
+my $LIMIT = 1;
+my $nr_running = 0;
+
 # n.b. serving "description" and "cloneurl" should be innocuous enough to
 # not cause problems.  serving "config" might...
 my @text = qw[HEAD info/refs
@@ -30,12 +39,20 @@ sub r {
 
 sub serve {
        my ($cgi, $git, $path) = @_;
+       return serve_dumb($cgi, $git, $path) if $nr_running >= $LIMIT;
+
        my $service = $cgi->param('service') || '';
        if ($service =~ /\Agit-\w+-pack\z/ || $path =~ /\Agit-\w+-pack\z/) {
                my $ok = serve_smart($cgi, $git, $path);
                return $ok if $ok;
        }
 
+       serve_dumb($cgi, $git, $path);
+}
+
+sub serve_dumb {
+       my ($cgi, $git, $path) = @_;
+
        my $type;
        if ($path =~ /\A(?:$BIN)\z/o) {
                $type = 'application/octet-stream';
@@ -73,11 +90,12 @@ sub serve {
                my $n = 8192;
                while ($len > 0) {
                        $n = $len if $len < $n;
-                       my $r = read($in, $buf, $n);
+                       my $r = sysread($in, $buf, $n);
                        last if (!defined($r) || $r <= 0);
                        $len -= $r;
                        $fh->write($buf);
                }
+               die "$f truncated with $len bytes remaining\n" if $len;
                $fh->close;
        }
 }
@@ -132,18 +150,19 @@ sub serve_smart {
        my $buf;
        my $in;
        my $err = $env->{'psgi.errors'};
-       if (fileno($input) >= 0) {
+       my $fd = eval { fileno($input) };
+       if (defined $fd && $fd >= 0) {
                $in = $input;
-       } else { # FIXME untested
+       } else {
                $in = input_to_file($env) or return r(500);
        }
        my ($rpipe, $wpipe);
        unless (pipe($rpipe, $wpipe)) {
-               $err->print("error creating pipe: $!\n");
-               return r(500);
+               $err->print("error creating pipe: $! - going static\n");
+               return;
        }
        my %env = %ENV;
-       # GIT_HTTP_EXPORT_ALL, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
+       # GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL
        # may be set in the server-process and are passed as-is
        foreach my $name (qw(QUERY_STRING
                                REMOTE_USER REMOTE_ADDR
@@ -160,43 +179,52 @@ sub serve_smart {
        my %rdr = ( 0 => fileno($in), 1 => fileno($wpipe) );
        my $pid = spawn([qw(git http-backend)], \%env, \%rdr);
        unless (defined $pid) {
-               $err->print("error spawning: $!\n");
-               return r(500);
+               $err->print("error spawning: $! - going static\n");
+               return;
        }
        $wpipe = $in = undef;
        $buf = '';
        my ($vin, $fh, $res);
+       $nr_running++;
        my $end = sub {
                if ($fh) {
                        $fh->close;
                        $fh = undef;
-               } else {
-                       $res->(r(500)) if $res;
                }
                if ($rpipe) {
-                       $rpipe->close; # _may_ be Danga::Socket::close
+                       # _may_ be Danga::Socket::close via
+                       # PublicInbox::HTTPD::Async::close:
+                       $rpipe->close;
                        $rpipe = undef;
+                       $nr_running--;
                }
                if (defined $pid) {
-                       my $wpid = $pid;
-                       $pid = undef;
-                       return if $wpid == waitpid($wpid, 0);
-                       $err->print("git http-backend ($git_dir): $?\n");
+                       my $e = $pid == waitpid($pid, 0) ?
+                               $? : "PID:$pid still running?";
+                       if ($e) {
+                               $err->print("http-backend ($git_dir): $e\n");
+                               if (my $io = $env->{'psgix.io'}) {
+                                       $io->close;
+                               }
+                       }
                }
+               return unless $res;
+               my $dumb = serve_dumb($cgi, $git, $path);
+               ref($dumb) eq 'ARRAY' ? $res->($dumb) : $dumb->($res);
        };
        my $fail = sub {
-               my ($e) = @_;
-               if ($e eq 'EAGAIN') {
+               if ($!{EAGAIN} || $!{EINTR}) {
                        select($vin, undef, undef, undef) if defined $vin;
                        # $vin is undef on async, so this is a noop on EAGAIN
                        return;
                }
+               my $e = $!;
                $end->();
                $err->print("git http-backend ($git_dir): $e\n");
        };
        my $cb = sub { # read git-http-backend output and stream to client
                my $r = $rpipe ? $rpipe->sysread($buf, 8192, length($buf)) : 0;
-               return $fail->($!{EAGAIN} ? 'EAGAIN' : $!) unless defined $r;
+               return $fail->() unless defined $r;
                return $end->() if $r == 0; # EOF
                if ($fh) { # stream body from git-http-backend to HTTP client
                        $fh->write($buf);
@@ -208,22 +236,27 @@ sub serve_smart {
                        foreach my $l (split(/\r\n/, $h)) {
                                my ($k, $v) = split(/:\s*/, $l, 2);
                                if ($k =~ /\AStatus\z/i) {
-                                       $code = int($v);
+                                       ($code) = ($v =~ /\b(\d+)\b/);
                                } else {
                                        push @h, $k, $v;
                                }
                        }
-                       # write response header:
-                       $fh = $res->([ $code, \@h ]);
-                       $res = undef;
-                       $fh->write($buf);
+                       if ($code == 403) {
+                               # smart cloning disabled, serve dumbly
+                               # in $end since we never undef $res in here
+                       } else { # write response header:
+                               $fh = $res->([ $code, \@h ]);
+                               $res = undef;
+                               $fh->write($buf);
+                       }
                        $buf = '';
                } # else { keep reading ... }
        };
        if (my $async = $env->{'pi-httpd.async'}) {
+               # $async is PublicInbox::HTTPD::Async->new($rpipe, $cb)
                $rpipe = $async->($rpipe, $cb);
                sub { ($res) = @_ } # let Danga::Socket handle the rest.
-       } else { # synchronous loop
+       } else { # synchronous loop for other PSGI servers
                $vin = '';
                vec($vin, fileno($rpipe), 1) = 1;
                sub {
@@ -233,7 +266,6 @@ sub serve_smart {
        }
 }
 
-# FIXME: untested, our -httpd _always_ gives a real file handle
 sub input_to_file {
        my ($env) = @_;
        my $in = IO::File->new_tmpfile;