diff options
author | Eric Wong <e@80x24.org> | 2018-12-06 02:40:06 +0000 |
---|---|---|
committer | Eric Wong <e@80x24.org> | 2018-12-06 03:42:55 +0000 |
commit | 74923a7cb6ace0369455e2155cbf63dfa4be33ea (patch) | |
tree | a28d35e1b71a711265a0f2d9d1f1b8bc49e3130b /lib | |
parent | fbfd63996ddfc9a23b8a1e0f8756e378f2e196ba (diff) | |
download | public-inbox-74923a7cb6ace0369455e2155cbf63dfa4be33ea.tar.gz |
When a client starts pipelining requests to us which trigger long responses, we need to keep socket readiness checks disabled and only enable them when our socket rbuf is drained. Failure to do this caused aborted clients with "BUG: nested long response" when Danga::Socket calls event_read for read-readiness after our "next_tick" sub fires in the same event loop iteration. Reported-by: Jonathan Corbet <corbet@lwn.net> cf. https://public-inbox.org/meta/20181013124658.23b9f9d2@lwn.net/
Diffstat (limited to 'lib')
-rw-r--r-- | lib/PublicInbox/NNTP.pm | 26 |
1 files changed, 18 insertions, 8 deletions
diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm index 022bb809..90a5a3a5 100644 --- a/lib/PublicInbox/NNTP.pm +++ b/lib/PublicInbox/NNTP.pm @@ -51,8 +51,16 @@ sub next_tick () { # before finishing reading: if (my $long_cb = $nntp->{long_res}) { $nntp->write($long_cb); - } elsif (&Danga::Socket::POLLIN & $nntp->{event_watch}) { + } else { + # pipelined request, we bypassed socket-readiness + # checks to get here: event_read($nntp); + + # maybe there's more pipelined data, or we'll have + # to register it for socket-readiness notifications + if (!$nntp->{long_res} && !$nntp->{closed}) { + check_read($nntp); + } } } } @@ -609,7 +617,7 @@ sub long_response ($$) { now() - $t0); } else { update_idle_time($self); - $self->watch_read(1); + check_read($self); } } elsif ($more) { # $self->{write_buf_size}: # no recursion, schedule another call ASAP @@ -620,7 +628,7 @@ sub long_response ($$) { $nextt ||= PublicInbox::EvCleanup::asap(*next_tick); } else { # all done! $self->{long_res} = undef; - $self->watch_read(1); + check_read($self); res($self, '.'); out($self, " deferred[$fd] done - %0.6f", now() - $t0); } @@ -968,10 +976,9 @@ sub event_read { update_idle_time($self); } -sub watch_read { - my ($self, $bool) = @_; - my $rv = $self->SUPER::watch_read($bool); - if ($bool && index($self->{rbuf}, "\n") >= 0) { +sub check_read { + my ($self) = @_; + if (index($self->{rbuf}, "\n") >= 0) { # Force another read if there is a pipelined request. # We don't know if the socket has anything for us to read, # and we must double-check again by the time the timer fires @@ -979,8 +986,11 @@ sub watch_read { # another long response. push @$nextq, $self; $nextt ||= PublicInbox::EvCleanup::asap(*next_tick); + } else { + # no pipelined requests available, let the kernel know + # to wake us up if there's more + $self->watch_read(1); # Danga::Socket::watch_read } - $rv; } sub not_idle_long ($$) { |