On Wed, Aug 13, 2014 at 11:47:58AM +0300, Niko Tyni wrote: > On Wed, Aug 13, 2014 at 09:12:11AM +0100, Steve Hay wrote: > > > Thanks for the patch. I haven't seen this failure myself on Windows, > > but the patch certainly doesn't seem to break anything. I'm a little > > uneasy about it, though. It seems odd to structure read3.pm > > differently to read2.pm and read4.pm, which still have a plan emitted > > before the first read(). > > > > Why does read3.pm deadlock but read2.pm and read4.pm don't? If it's > > just luck then perhaps read2.pm and read4.pm should be changed > > similarly? > > It only happens with read3 because the POSTed string in that test is > very much larger than the other ones. LWP::Protocol::http sends the > whole POST request in one write() unless it's over eight kilobytes long > (see lib/LWP/Protocol/http.pm in libwww-perl_6.08 line 276.)
I've updated the attached patch to change read2 too. read4 is quite different though, as it reads just a few bytes at a time, so I think it's OK to leave it as is. > > Maybe a comment in the code would be good too, since the structure of > > most test files is to get the plan out early. Otherwise there is scope > > for re-introducing the bug in the future since it isn't obvious that > > the placement of the plan is important. Done as well. Thanks again, -- Niko Tyni nt...@debian.org
>From 2eb936dd9c55e8e3500ff9c0277f9c93dcf357c6 Mon Sep 17 00:00:00 2001 From: Niko Tyni <nt...@debian.org> Date: Fri, 8 Aug 2014 17:22:33 +0000 Subject: Don't answer anything to the client before it's done sending the POST data LWP::Protocol::http sends long POST requests with several write() calls. If it gets a response with a full HTTP header (up to the two newlines) before it's done with the later write() calls sending the actual POST data, it will stop writing. This leads to a deadlock and a test failure after timeout. The problem only shows up with large POSTED strings like the one in read3.pm, but update read2.pm too for correctness. Thread at http://mail-archives.apache.org/mod_mbox/perl-dev/201408.mbox/%3C20140809104131.GA3670@estella.local.invalid%3E Bug-Debian: https://bugs.debian.org/697682 --- t/response/TestApache/read2.pm | 6 ++++-- t/response/TestApache/read3.pm | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/t/response/TestApache/read2.pm b/t/response/TestApache/read2.pm index 4307181..4cf7348 100644 --- a/t/response/TestApache/read2.pm +++ b/t/response/TestApache/read2.pm @@ -20,8 +20,6 @@ my $expected = "foobar"; sub handler { my $r = shift; - plan $r, tests => 1; - # test the case where the buffer to be filled has set magic # attached. which is the case when one passes an non-existing hash # entry value. it's not autovivified when passed to the function @@ -30,6 +28,10 @@ sub handler { my $data; my $len = $r->read($data->{buffer}, $r->headers_in->{'Content-Length'}); + # only print the plan out after reading to avoid chances of a deadlock + # see http://mail-archives.apache.org/mod_mbox/perl-dev/201408.mbox/%3C20140809104131.GA3670@estella.local.invalid%3E + plan $r, tests => 1; + ok t_cmp($data->{buffer}, $expected, "reading into an autovivified hash entry"); diff --git a/t/response/TestApache/read3.pm b/t/response/TestApache/read3.pm index 8ad9f26..b1522d5 100644 --- a/t/response/TestApache/read3.pm +++ b/t/response/TestApache/read3.pm @@ -20,8 +20,6 @@ my $expected = "foobar"x2000; sub handler { my $r = shift; - plan $r, tests => 1; - # test to read data up to end of file is signaled my $data = ''; my $where = 0; @@ -31,6 +29,10 @@ sub handler { $where += $len; } while ($len > 0); + # only print the plan out after reading to avoid chances of a deadlock + # see http://mail-archives.apache.org/mod_mbox/perl-dev/201408.mbox/%3C20140809104131.GA3670@estella.local.invalid%3E + plan $r, tests => 1; + ok t_cmp($data, $expected, "reading up to end of file"); Apache2::Const::OK;
