| Subject: | PostHandler only once. Better connection_close (with tests) |
This patch will bring 0.09 up to date with my internal version of
PoCo::Server::HTTP, including all the fixes that I've had to implement
since 0.07.
The included patch makes sure that PostHandler is only called once per
request. Previously, it could be called multiple times if an error
occurred. With test case.
It also includes a work around where some versions of POE and perl don't
call DESTROY on a deleted wheel.
It also tweaks the POD (20_stream.t not 30_stream.t)
Also, some tweaks to the test cases.
| Subject: | Philip_Gwyn.Queue-cleanup.connection_close.01.patch |
diff -rub POE-Component-Server-HTTP-0.09/lib/POE/Component/Server/HTTP.pm POE-Component-Server-HTTP-0.09-PG/lib/POE/Component/Server/HTTP.pm
--- POE-Component-Server-HTTP-0.09/lib/POE/Component/Server/HTTP.pm 2006-05-23 16:45:07.000000000 -0400
+++ POE-Component-Server-HTTP-0.09-PG/lib/POE/Component/Server/HTTP.pm 2007-08-22 23:44:20.000000000 -0400
@@ -98,6 +98,9 @@
my ($kernel, $session, $heap) = @_[KERNEL, SESSION, HEAP];
$kernel->call($tcp_alias, "shutdown");
$kernel->alias_remove($alias);
+ foreach my $id (keys %{$heap->{wheels}}) {
+ close_connection($heap, $id);
+ }
},
},
heap => { self => $self }
@@ -134,17 +137,22 @@
}
sub error_queue {
- return [qw(
- Map
- ErrorHandler
- PostHandler
- Cleanup
- )];
+ my($self, $c, $pre)=@_;
+
+ # If client closed connect, we don't want PostHandler called twice
+ if( $c->{client_closed} and $c->{response_sent} and not $c->{post_done} ) {
+ warn "Client closed, response_sent, but PostHandler not called?";
+ }
+ return [qw(Map ErrorHandler Cleanup)]
+ if $c->{client_closed} and $c->{post_done};
+ return [qw( Map PreHandler ErrorHandler PostHandler Cleanup )] if $pre;
+ return [qw( Map ErrorHandler PostHandler Cleanup )];
}
# Set up queue for handling this request
sub rebuild_queue {
- my( $self, $handlers) = @_;
+ my( $self, $c) = @_;
+ my $handlers = $c->{handlers};
my $now = $handlers->{Queue}[0]; # what phase are we about to do?
if (not $now) { # this means we are post Cleanup
@@ -154,11 +162,13 @@
$handlers->{Queue} = ['Map', 'ErrorHandler', 'Cleanup'];
# Note : sub error set up fake request/response objects, etc
}
- elsif ($now eq 'TransHandler' or $now eq 'Map' or
- $now eq 'PreHandler' or $now eq 'ContentHandler' or
+ elsif ($now eq 'TransHandler' or $now eq 'Map' ) {
+ $handlers->{Queue}=$self->error_queue($c, 1);
+ }
+ elsif ($now eq 'PreHandler' or $now eq 'ContentHandler' or
$now eq 'Send' or $now eq 'PostHandler') {
- $handlers->{Queue}=$self->error_queue;
+ $handlers->{Queue}=$self->error_queue($c);
}
elsif ($now eq 'Cleanup') {
# we need Map to turn set up ErrorHandler
@@ -170,6 +180,23 @@
$handlers->{PreHandler} = [];
}
+sub close_connection {
+ my($heap, $id)=@_;
+
+ DEBUG and warn "Close connection";
+ my $connection=$heap->{c}->{$id};
+
+ delete($connection->{handlers});
+ delete($connection->{wheel});
+ delete($heap->{c}->{$id});
+
+ my $w=delete $heap->{wheels}->{$id};
+ # WORK AROUND: Some versions of Perl and POE don't DESTROY wheels properly
+ $w->DESTROY if $w;
+ return;
+}
+
+
sub accept {
my ($socket,$remote_addr, $remote_port) = @_[ARG0, ARG1, ARG2];
my $self = $_[HEAP]->{self};
@@ -184,8 +211,10 @@
ContentHandler => undef,
PostHandler => [],
# IMHO, Queue should be set in 'input' --PG
+ # 2006/01 -- no, because rebuild_queue needs to see it set --PG
Queue => $self->handler_queue,
};
+ $connection->{post_done} = 0;
my $wheel = POE::Wheel::ReadWrite->new(
Handle => $socket,
@@ -271,8 +300,11 @@
$c->{request}->is_error(1);
$c->{response}->is_error(1);
+ # mark this connection as closed
+ $c->{client_closed}=1 if $op eq 'read' and $errnum==0;
+
# and rebuild the queue
- $self->rebuild_queue($c->{handlers});
+ $self->rebuild_queue($c);
$poe_kernel->yield('execute',$id);
}
}
@@ -306,12 +338,13 @@
if ($state eq 'Map') {
$self->state_Map( $request->uri ? $request->uri->path : '',
- $handlers, $request );
+ $handlers, $request, $connection );
shift @{$handlers->{Queue}};
next;
}
elsif ($state eq 'Send') {
- $self->state_Send( $response, $_[HEAP]->{wheels}->{$id} );
+ $self->state_Send( $response, $_[HEAP]->{wheels}->{$id},
+ $connection );
shift @{$handlers->{Queue}};
last;
}
@@ -365,10 +398,7 @@
}
else {
DEBUG and warn "Close connection";
- delete($connection->{handlers});
- delete($connection->{wheel});
- delete($_[HEAP]->{c}->{$id});
- delete($_[HEAP]->{wheels}->{$id});
+ close_connection($_[HEAP], $id);
}
last HANDLERS;
}
@@ -376,6 +406,9 @@
$self->{StreamHandler}->($request, $response);
last HANDLERS;
}
+ elsif( $state eq 'PostHandler' ) {
+ $connection->{post_done} = 1;
+ }
DISPATCH: # this is used for {Trans,Pre,Post}Handler
while (1) {
@@ -397,10 +430,7 @@
}
sub state_Map {
- my $self = shift;
- my $path = shift;
- my $handlers = shift;
- my $request = shift;
+ my( $self, $path, $handlers, $request, $c ) = @_;
my $filename;
(undef, $path,$filename) = File::Spec->splitpath($path);
my @dirs = File::Spec->splitdir($path);
@@ -420,11 +450,10 @@
DEBUG and warn "check=", join ',', @check;
my @todo;
- unless ($request->is_error) {
- @todo=qw(PreHandler ContentHandler PostHandler);
- }
- else {
- @todo=qw(ErrorHandler PostHandler);
+ # We need to map all the Handler phases in the queue
+ foreach my $phase ( @{ $c->{handlers}{Queue} } ) {
+ next unless $phase =~ /Handler$/;
+ push @todo, $phase
}
foreach my $path (@check) {
@@ -443,9 +472,9 @@
}
sub state_Send {
- my $self = shift;
- my $response = shift;
- my $wheel = shift;
+ my($self, $response, $wheel, $c) = @_;
+
+ $c->{response_sent}=1;
$response->header(%{$self->{Headers}});
unless ($response->header('Date')) {
@@ -647,7 +676,7 @@
}
}
-Another example can be found in t/30_stream.t. The parts dealing with
+Another example can be found in t/20_stream.t. The parts dealing with
multipart/mixed are well documented and at the end of the file.
NOTE: Changes in streaming mode are only verified when StreamHandler exits.
Only in POE-Component-Server-HTTP-0.09-PG/lib/POE/Component/Server: HTTP.pm~
diff -rub POE-Component-Server-HTTP-0.09/t/30_error.t POE-Component-Server-HTTP-0.09-PG/t/30_error.t
--- POE-Component-Server-HTTP-0.09/t/30_error.t 2006-05-23 16:10:35.000000000 -0400
+++ POE-Component-Server-HTTP-0.09-PG/t/30_error.t 2007-08-22 23:41:55.000000000 -0400
@@ -27,7 +27,7 @@
####################################################################
unless ($pid) { # we are child
- Test::Builder->new->no_ending(1);
+ Test::More->builder->no_ending(1);
# stop kernel from griping
${$poe_kernel->[POE::Kernel::KR_RUN]} |=
POE::Kernel::KR_RUN_CALLED;
@@ -76,16 +76,20 @@
####################################################################
# we are the parent
-plan tests=>11;
+plan tests=>16;
my $Q=1;
my $shutdown=0;
my $top=0;
my $bonk=0;
+my $connected=0;
my $aliases = POE::Component::Server::HTTP->new(
Port => $PORT,
Address=>'localhost',
+ PreHandler => {
+ '/' => \&pre_top,
+ },
ContentHandler => { '/' => \&top,
'/honk/shutdown.html' => \&shutdown,
'/bonk/' => \&bonk
@@ -143,6 +147,13 @@
}
#######################################
+sub pre_top
+{
+ DEBUG and warn "pre_top";
+ $connected++;
+}
+
+#######################################
sub top
{
my ($request, $response) = @_;
@@ -170,8 +181,12 @@
sub post_top
{
my($request, $response)=@_;
+ DEBUG and warn "post_top";
+
+ $connected--;
ok(($shutdown or (not $bonk and $request->is_error)),
"all but shutdown requests should be errors");
+ ok(0==$connected, "Pre was called ($connected)");
}
#######################################
@@ -179,15 +194,16 @@
{
my($request, $response)=@_;
ok($shutdown, "we are after shutdown");
+ ok(0==$connected, "Pre/Post was called once ($connected)");
}
#######################################
sub shutdown
{
my ($request, $response) = @_;
- DEBUG and warn "SHUTDOWN";
- $poe_kernel->post($aliases->{httpd} => 'shutdown');
- $poe_kernel->post($aliases->{tcp} => 'shutdown');
+
+ $poe_kernel->state( do_shutdown => \&do_shutdown );
+ $poe_kernel->delay( 'do_shutdown', 1 );
$shutdown=1;
@@ -197,6 +213,15 @@
return RC_OK;
}
+#######################################
+sub do_shutdown
+{
+ DEBUG and warn "DOING SHUTDOWN";
+ $poe_kernel->post($aliases->{httpd} => 'shutdown');
+ $poe_kernel->post($aliases->{tcp} => 'shutdown');
+
+}
+
sub __peek
{
my($verbose)=@_;
Only in POE-Component-Server-HTTP-0.09-PG/t: 30_error.t~