Bug #14368 for POE-Component-Client-HTTP: PoCo::Client::HTTP "eats" requests in some situations

Tue Aug 30 13:13:52 2005 YKAR [...] cpan.org - Ticket created

Subject:

PoCo::Client::HTTP "eats" requests in some situations

There is situation in which PoCo::Client::HTTP does not send response to client. I found if PoCo::Client::HTTP successfully connects to host, send successfuly request and wait for response but response never sent. Then connection times out and no response sent back to client (which send request to PoCo::Client::HTTP). I wrote a patch which works for me, however I do not understand how to work with Client::Keepalive well. So please review patch thoroughly. Thank you in advance!

Index: HTTP.pm =================================================================== --- HTTP.pm (revision 211) +++ HTTP.pm (working copy) @@ -296,7 +296,7 @@ _finish_request($heap, $request, 0); return; } - elsif ($request->[REQ_STATE] & RS_POSTED) { + else { DEBUG and warn "I/O: Disconnect, keepalive timeout or HTTP/1.0."; $request->error(408, "Request timed out") if $request->[REQ_STATE]; return;

Sun Sep 04 10:47:42 2005 YKAR [...] cpan.org - Correspondence added

[YKAR - Tue Aug 30 13:13:52 2005]: The problem of my last patch (and not only problem of my patch, problem was originally) that it does not close the connection (and it returnetd implicitly to PoCoCl::Keepalive pool, because PoCoCl::HTTP::Request object was destroyed and it has reference to PoCo::Connection::Keepalive which on DESTROY returns socket to keepalive pool). So new connections tries to connect on timed out socket (and double HTTP request are sent, first from first request and second from second). I attaching second version of patch. It closes connection, but do many dirty work with PoCoCl::Keepalive internals. I did not found the interface to close (remove from keepalive pool) socket which has unrecoverable error (not connection error, but logical like connection timed out (hanged), or can't be returned to reusable state). Maybe you know how to close connection in better way? Second, basically the problem I mentioned in subject. I'm proposing to change line 299 of PoCoCl::HTTP from elsif ($request->[REQ_STATE] & RS_POSTED) { to else { Maybe need to change as following elsif ($request->[REQ_STATE] & RS_POSTED) { to elsif (not $request->[REQ_STATE] & RS_POSTED) { But "elsif ($request->[REQ_STATE] & RS_POSTED)" is definetly wrong. Because it after calls: $request->error(408, "Request timed out") if $request->[REQ_STATE]; (Request::error does postback to client, and RS_POSTED means that postback already posted to client). So logical error here checked if postback posted and send it again? hm? So maybe missed not and "elsif ($request->[REQ_STATE] & RS_POSTED)" must be read as "elsif (not $request->[REQ_STATE] & RS_POSTED)". So it will call error method only of postback not yet posted. Show quoted text

> There is situation in which PoCo::Client::HTTP does not send response > to client. > > I found if PoCo::Client::HTTP successfully connects to host, send > successfuly request and wait for response but response never sent. > Then connection times out and no response sent back to client > (which send request to PoCo::Client::HTTP). > > I wrote a patch which works for me, however I do not understand how to > work with Client::Keepalive well. So please review patch > thoroughly. > > Thank you in advance!

Index: HTTP.pm =================================================================== --- HTTP.pm (revision 212) +++ HTTP.pm (working copy) @@ -296,8 +296,16 @@ _finish_request($heap, $request, 0); return; } - elsif ($request->[REQ_STATE] & RS_POSTED) { + else { DEBUG and warn "I/O: Disconnect, keepalive timeout or HTTP/1.0."; + # I don't know how to close timed out socket properly + # I hope there is better solution for this + # <Close socket> (Modifying PoCoCl::Keepalive guts) + my $socket = $request->[REQ_CONNECTION]->[POE::Component::Connection::Keepalive::CK_SOCKET]; + my $used = delete $heap->{cm}->[POE::Component::Client::Keepalive::SF_USED]->{$socket}; + my $request_key = $used->[POE::Component::Client::Keepalive::USED_KEY]; + $heap->{cm}->_decrement_used_each($request_key); + # </Close socket> $request->error(408, "Request timed out") if $request->[REQ_STATE]; return; }

Sun Sep 04 14:27:09 2005 YKAR [...] cpan.org - Correspondence added

[YKAR - Tue Aug 30 13:13:52 2005]: Attaching test case for this situation. Show quoted text

> There is situation in which PoCo::Client::HTTP does not send response > to client. > > I found if PoCo::Client::HTTP successfully connects to host, send > successfuly request and wait for response but response never sent. > Then connection times out and no response sent back to client > (which send request to PoCo::Client::HTTP). > > I wrote a patch which works for me, however I do not understand how to > work with Client::Keepalive well. So please review patch > thoroughly. > > Thank you in advance!

#! /usr/bin/perl # -*- perl -*- use strict; use warnings; use Test::More tests => 1; use POE; use POE::Component::Client::HTTP; use POE::Component::Server::TCP; use HTTP::Request::Common qw(GET); use Socket; POE::Component::Client::HTTP->spawn ( Alias => 'ua', Timeout => 1 ); POE::Session->create ( inline_states => { _start => sub { my ($kernel) = $_[KERNEL]; $kernel->alias_set('Main'); # Spawn discard TCP server POE::Component::Server::TCP->new ( Alias => 'Discard', Address => '127.0.0.1', Port => 0, ClientInput => sub {}, # discard Started => sub { my ($kernel, $heap) = @_[KERNEL, HEAP]; my $port = (sockaddr_in($heap->{listener}->getsockname))[0]; $kernel->post('Main', 'set_port', $port); } ); }, set_port => sub { my ($kernel, $port) = @_[KERNEL, ARG0]; my $url = "http://127.0.0.1:$port/"; $kernel->post(ua => request => response => GET $url); $kernel->delay(no_response => 10); }, response => sub { my ($kernel, $rspp) = @_[KERNEL, ARG1]; my $rsp = $rspp->[0]; $kernel->delay('no_response'); # Clear timer ok($rsp->code == 408); $kernel->post(Discard => 'shutdown'); }, no_response => sub { my $kernel = $_[KERNEL]; fail(); $kernel->post(Discard => 'shutdown'); } } ); POE::Kernel->run;

Sun Sep 04 15:04:51 2005 RCAPUTO [...] cpan.org - Correspondence added

I made a change that is the equivalent of your patch and also applied your test case. Thank you again.

Sun Sep 04 15:04:52 2005 RCAPUTO [...] cpan.org - Status changed from 'new' to 'resolved'