[PATCH 0/2] httpd/async race fixes

[PATCH 0/2] httpd/async race fixes

[Eric Wong]

Oops, these are some tricky fixes which took a while to notice
and find during stress testing.

Eric Wong (2):
      httpd/async: do not attempt future writes on closed sockets
      httpd/async: reinstate D::S timer usage for cleanup

 lib/PublicInbox/EvCleanup.pm   | 9 +++++++++
 lib/PublicInbox/HTTPD/Async.pm | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

[PATCH 1/2] httpd/async: do not attempt future writes on closed sockets

[Eric Wong]

Danga::Socket::close does not clear the write_buf_size field,
so it's conceivable we could attempt to queue up data and
callbacks we can never flush out.
---
 lib/PublicInbox/HTTPD/Async.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index a936d9b..880cf02 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -41,6 +41,7 @@ sub async_pass {
 		my $r = sysread($self->{sock}, $$bref, 8192);
 		if ($r) {
 			$fh->write($$bref);
+			return if $io->{closed};
 			if ($io->{write_buf_size}) {
 				$self->watch_read(0);
 				$io->write($restart_read); # D::S::write
-- 
EW

[PATCH 2/2] httpd/async: reinstate D::S timer usage for cleanup

[Eric Wong]

EvCleanup::asap events are not guaranteed to run after
Danga::Socket closes sockets at the event loop.  Thus we
must use slower Danga::Socket timers which are guaranteed
to run at the end of the event loop.
---
 lib/PublicInbox/EvCleanup.pm   | 9 +++++++++
 lib/PublicInbox/HTTPD/Async.pm | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/EvCleanup.pm b/lib/PublicInbox/EvCleanup.pm
index 61837b8..2b77c61 100644
--- a/lib/PublicInbox/EvCleanup.pm
+++ b/lib/PublicInbox/EvCleanup.pm
@@ -9,6 +9,7 @@ use base qw(Danga::Socket);
 use fields qw(rd);
 my $singleton;
 my $asapq = [ [], undef ];
+my $nextq = [ [], undef ];
 my $laterq = [ [], undef ];
 
 sub once_init () {
@@ -30,6 +31,7 @@ sub _run_all ($) {
 }
 
 sub _run_asap () { _run_all($asapq) }
+sub _run_next () { _run_all($nextq) }
 sub _run_later () { _run_all($laterq) }
 
 # Called by Danga::Socket
@@ -51,6 +53,12 @@ sub asap ($) {
 	$asapq->[1] ||= _asap_timer();
 }
 
+sub next_tick ($) {
+	my ($cb) = @_;
+	push @{$nextq->[0]}, $cb;
+	$nextq->[1] ||= Danga::Socket->AddTimer(0, *_run_next);
+}
+
 sub later ($) {
 	my ($cb) = @_;
 	push @{$laterq->[0]}, $cb;
@@ -59,6 +67,7 @@ sub later ($) {
 
 END {
 	_run_asap();
+	_run_next();
 	_run_later();
 }
 
diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index 880cf02..68514f5 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -72,7 +72,7 @@ sub close {
 	$self->SUPER::close(@_);
 
 	# we defer this to the next timer loop since close is deferred
-	PublicInbox::EvCleanup::asap($cleanup) if $cleanup;
+	PublicInbox::EvCleanup::next_tick($cleanup) if $cleanup;
 }
 
 1;
-- 
EW