about summary refs log tree commit
diff options
context:
space:
mode:
authorEric Wong <e@80x24.org>2019-07-10 06:13:59 +0000
committerEric Wong <e@80x24.org>2019-07-10 08:25:06 +0000
commit4685b1d88ffe1f18334bfdd12977ece1fe9d11ce (patch)
tree745eb7d4b0852f7eed8b1e1d50bbfb96df37eddb
parentd5615a3c28aaaa86385c400b01050177b60469a2 (diff)
downloadpublic-inbox-4685b1d88ffe1f18334bfdd12977ece1fe9d11ce.tar.gz
http|nntp: avoid recursion inside ->write
In HTTP.pm, we can use the same technique NNTP.pm uses with
long_response with the $long_cb callback and avoid storing
$pull in the per-client structure at all.  We can also reuse
the same logic to push the callback into wbuf from NNTP.

This does NOT introduce a new circular reference, but documents
it more clearly.
-rw-r--r--lib/PublicInbox/HTTP.pm64
-rw-r--r--lib/PublicInbox/NNTP.pm3
2 files changed, 34 insertions, 33 deletions
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 60b287c4..5afe167e 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -11,7 +11,7 @@ package PublicInbox::HTTP;
 use strict;
 use warnings;
 use base qw(PublicInbox::DS);
-use fields qw(httpd env input_left remote_addr remote_port forward pull);
+use fields qw(httpd env input_left remote_addr remote_port forward);
 use bytes (); # only for bytes::length
 use Fcntl qw(:seek);
 use Plack::HTTPParser qw(parse_http_request); # XS or pure Perl
@@ -260,48 +260,49 @@ sub response_done_cb ($$) {
         }
 }
 
-sub getline_cb ($$$) {
+sub getline_response ($$$) {
         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 };
+        my $pull; # DANGER: self-referential
+        $pull = sub {
+                my $forward = $self->{forward};
+                # limit our own running time for fairness with other
+                # clients and to avoid buffering too much:
+                my $buf = eval {
+                        local $/ = \8192;
+                        $forward->getline;
+                } if $forward;
+
                 if (defined $buf) {
                         $write->($buf); # may close in PublicInbox::DS::write
+
                         if ($self->{sock}) {
-                                my $next = $self->{pull};
-                                if ($self->{wbuf}) {
-                                        $self->write($next);
-                                } else {
-                                        PublicInbox::DS::requeue($next);
-                                }
-                                return;
+                                my $wbuf = $self->{wbuf} ||= [];
+                                push @$wbuf, $pull;
+
+                                # wbuf may be populated by $write->($buf),
+                                # no need to rearm if so:
+                                $self->requeue if scalar(@$wbuf) == 1;
+                                return; # likely
                         }
                 } elsif ($@) {
                         err($self, "response ->getline error: $@");
-                        $forward = undef;
                         $self->close;
                 }
-        }
 
-        delete @$self{qw(forward pull)};
-        # avoid recursion
-        if ($forward) {
-                eval { $forward->close };
-                if ($@) {
-                        err($self, "response ->close error: $@");
-                        $self->close; # idempotent
+                $pull = undef; # all done!
+                # avoid recursion
+                if (delete $self->{forward}) {
+                        eval { $forward->close };
+                        if ($@) {
+                                err($self, "response ->close error: $@");
+                                $self->close; # idempotent
+                        }
                 }
-        }
-        $close->();
-}
+                $forward = undef;
+                $close->(); # call response_done_cb
+        };
 
-sub getline_response ($$$) {
-        my ($self, $write, $close) = @_;
-        my $pull = $self->{pull} = sub { getline_cb($self, $write, $close) };
-        $pull->();
+        $pull->(); # kick-off!
 }
 
 sub response_write {
@@ -453,7 +454,6 @@ sub close {
         if (my $env = delete $self->{env}) {
                 delete $env->{'psgix.io'}; # prevent circular references
         }
-        delete $self->{pull};
         if (my $forward = delete $self->{forward}) {
                 eval { $forward->close };
                 err($self, "forward ->close error: $@") if $@;
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 6796a3c4..f4208f87 100644
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -658,7 +658,8 @@ sub long_response ($$) {
                         $long_cb = undef;
                         res($self, '.');
                         out($self, " deferred[$fd] done - %0.6f", now() - $t0);
-                        $self->requeue unless $self->{wbuf};
+                        my $wbuf = $self->{wbuf};
+                        $self->requeue unless $wbuf && @$wbuf;
                 }
         };
         $self->write($long_cb); # kick off!