wwwstream: improve documentation and variable naming

[public-inbox.git] / lib / PublicInbox / HTTP.pm

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 17f89302bceca8e24180f8ddbfc0931bf77220f2..cac14be380874b03df90a2cf4e54b9bb816a6c86 100644 (file)
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -17,7 +17,7 @@ use Plack::HTTPParser qw(parse_http_request); # XS or pure Perl
 use HTTP::Status qw(status_message);
 use HTTP::Date qw(time2str);
 use Scalar::Util qw(weaken);
-use IO::File;
+use IO::Handle;
 use constant {
        CHUNK_START => -1,   # [a-f0-9]+\r\n
        CHUNK_END => -2,     # \r\n
@@ -43,7 +43,7 @@ sub process_pipelineq () {
 our $MAX_REQUEST_BUFFER = $ENV{GIT_HTTP_MAX_REQUEST_BUFFER} ||
                        (10 * 1024 * 1024);
 
-my $null_io = IO::File->new('/dev/null', '<');
+open(my $null_io, '<', '/dev/null') or die "failed to open /dev/null: $!";
 my $http_date;
 my $prev = 0;
 sub http_date () {
@@ -245,29 +245,49 @@ sub response_done ($$) {
        $self->write(sub { $alive ? next_request($self) : $self->close });
 }
 
+sub getline_cb ($$$) {
+       my ($self, $write, $close) = @_;
+       local $/ = \8192;
+       my $forward = $self->{forward};
+       # limit our own running time for fairness with other
+       # clients and to avoid buffering too much:
+       if ($forward) {
+               my $buf = eval { $forward->getline };
+               if (defined $buf) {
+                       $write->($buf); # may close in Danga::Socket::write
+                       unless ($self->{closed}) {
+                               my $next = $self->{pull};
+                               if ($self->{write_buf_size}) {
+                                       $self->write($next);
+                               } else {
+                                       PublicInbox::EvCleanup::asap($next);
+                               }
+                               return;
+                       }
+               } elsif ($@) {
+                       err($self, "response ->getline error: $@");
+                       $forward = undef;
+                       $self->close;
+               }
+       }
+
+       $self->{forward} = $self->{pull} = undef;
+       # avoid recursion
+       if ($forward) {
+               eval { $forward->close };
+               if ($@) {
+                       err($self, "response ->close error: $@");
+                       $self->close; # idempotent
+               }
+       }
+       $close->();
+}
+
 sub getline_response {
        my ($self, $body, $write, $close) = @_;
        $self->{forward} = $body;
        weaken($self);
-       my $pull = $self->{pull} = sub {
-               local $/ = \8192;
-               my $forward = $self->{forward};
-               # limit our own running time for fairness with other
-               # clients and to avoid buffering too much:
-               while ($forward && defined(my $buf = $forward->getline)) {
-                       $write->($buf);
-                       last if $self->{closed};
-                       if ($self->{write_buf_size}) {
-                               $self->write($self->{pull});
-                       } else {
-                               PublicInbox::EvCleanup::asap($self->{pull});
-                       }
-                       return;
-               }
-               $self->{forward} = $self->{pull} = undef;
-               $forward->close if $forward; # avoid recursion
-               $close->();
-       };
+       my $pull = $self->{pull} = sub { getline_cb($self, $write, $close) };
        $pull->();
 }
 
@@ -315,10 +335,10 @@ sub input_prepare {
                        quit($self, 413);
                        return;
                }
-               $input = IO::File->new_tmpfile;
+               open($input, '+>', undef);
        } elsif (env_chunked($env)) {
                $len = CHUNK_START;
-               $input = IO::File->new_tmpfile;
+               open($input, '+>', undef);
        }
 
        # TODO: expire idle clients on ENFILE / EMFILE
@@ -331,12 +351,15 @@ sub input_prepare {
 
 sub env_chunked { ($_[0]->{HTTP_TRANSFER_ENCODING} || '') =~ /\bchunked\b/i }
 
+sub err ($$) {
+       eval { $_[0]->{httpd}->{env}->{'psgi.errors'}->print($_[1]."\n") };
+}
+
 sub write_err {
        my ($self, $len) = @_;
-       my $err = $self->{httpd}->{env}->{'psgi.errors'};
        my $msg = $! || '(zero write)';
-       $msg .= " ($len bytes remaining)\n" if defined $len;
-       $err->print("error buffering to input: $msg\n");
+       $msg .= " ($len bytes remaining)" if defined $len;
+       err($self, "error buffering to input: $msg");
        quit($self, 500);
 }
 
@@ -347,8 +370,7 @@ sub recv_err {
                $self->{input_left} = $len;
                return;
        }
-       my $err = $self->{httpd}->{env}->{'psgi.errors'};
-       $err->print("error reading for input: $! ($len bytes remaining)\n");
+       err($self, "error reading for input: $! ($len bytes remaining)");
        quit($self, 500);
 }
 
@@ -451,7 +473,10 @@ sub close {
        my $env = $self->{env};
        delete $env->{'psgix.io'} if $env; # prevent circular referernces
        $self->{pull} = $self->{forward} = $self->{env} = undef;
-       $forward->close if $forward;
+       if ($forward) {
+               eval { $forward->close };
+               err($self, "forward ->close error: $@") if $@;
+       }
        $self->SUPER::close(@_);
 }