Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

decremented exit guard when failed to write header #37

Merged
merged 2 commits into from

2 participants

@maedama

I think Twiggy fails to decrement exit_guard counter when, it fails to write response header (i.e EPIPE).
I have not written a test yet but this pull request should fix the problem. Please tell me if I am wrong in any ways.

@miyagawa
Owner

Can you write a test, or at least how to reproduce the problem? Thanks!

@maedama

Thanks for the response! I would write test and come back.

@maedama

Hi @miyagawa san I have added a test.

Let me explain problems I am facing in detail. I have implemented Chat Online Server with Twiggy::Prefork and Server::Starter.

In our production environment, we see IO Error when writing header on rare occasion.

What happens then, is that Twiggy's exit_guard fails to decrement as I have written previously. And When we try to restart a process by HUP signal or process reaches max-request-per-client, the process stops to accept new connection and wait until all request are served, but this condition is never met due to exit_guard decrement failure and process hangs forever.

@miyagawa miyagawa merged commit 761545e into from
@maedama

Thanks as lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 11, 2013
  1. @maedama
Commits on Oct 12, 2013
  1. @maedama
This page is out of date. Refresh to see the latest.
Showing with 79 additions and 0 deletions.
  1. +3 −0  lib/Twiggy/Server.pm
  2. +76 −0 t/anyevent_closed_connection.t
View
3  lib/Twiggy/Server.pm
@@ -369,6 +369,9 @@ sub _write_psgi_response {
local $@;
eval { $cv->send($_[0]->recv); 1 } or $cv->croak($@);
});
+ } else {
+ $self->{exit_guard}->end;
+ eval { $cv->send($_[0]->recv); 1 } or $cv->croak($@);
}
});
View
76 t/anyevent_closed_connection.t
@@ -0,0 +1,76 @@
+use strict;
+use warnings;
+use Test::More qw(no_diag);
+use Test::TCP;
+use IO::Socket::INET;
+use Plack::Loader;
+use AnyEvent::Handle;
+
+my $app = sub {
+ my $env = shift;
+ return sub {
+ my ($responder, $sock) = @_;
+ my $disconnected = AE::cv;
+
+ # Write response after client disconnection
+ my $handle = AnyEvent::Handle->new(
+ fh => $sock,
+ on_read => sub {},
+ on_eof => sub { $disconnected->send; },
+ on_error => sub {},
+ );
+
+ $disconnected->cb(sub {
+ undef $disconnected;
+ undef $handle;
+ shift->recv;
+ $responder->([
+ 200,
+ [ 'Content-Type' => 'text/plain', 'X_FOO' => "a" x 1_000_000 ], # Write large header to force EPIPE
+ [ 'hello' ]
+ ]);
+ });
+ }
+};
+
+my $server = Test::TCP->new(
+ code => sub {
+ my $port = shift;
+ my $server = Plack::Loader->load("Twiggy", port => $port, host => "127.0.0.1");
+ $server->run($app);
+ exit; # Suppress Test::TCP "child process does not block" warning
+ },
+ auto_start => 1,
+);
+
+request($server->port);
+
+kill 'QUIT' => $server->pid;
+my $hanged = 0;
+local $SIG{ALRM} = sub { $hanged = 1; kill 'TERM' => $server->pid; };
+alarm(5);
+waitpid($server->pid, 0);
+alarm(0);
+
+is $hanged, 0, "server should shut down";
+done_testing();
+
+sub request {
+ my $port = shift;
+ my $sock = IO::Socket::INET->new(
+ Proto => 'tcp',
+ PeerAddr => '127.0.0.1',
+ PeerPort => $port,
+ ) or die "Cannot open client socket: $!";
+ $sock->autoflush;
+
+ my $req = <<_END_;
+GET / HTTP/1.0
+Host: localhost:$port
+User-Agent: hogehoge
+
+_END_
+ $req =~ s/\n/\r\n/g;
+ $sock->print($req);
+ $sock->close;
+}
Something went wrong with that request. Please try again.