Re: Bug#697682: libapache2-mod-perl2: Intermittent FTBFS: t/apache/read3.t failure

On 9 August 2014 11:41, Niko Tyni <[email protected]> wrote:
> tag 697682 = patch
> thanks
>
> On Thu, Aug 07, 2014 at 04:32:08PM +0300, Niko Tyni wrote:
>> On Sat, Mar 30, 2013 at 03:26:55PM +0000, Dominic Hargreaves wrote:
>
>> > This doesn't seem to be limited to s390; I've seen it in on i386 during
>> > perl 5.16 rebuilds.
>>
>> As a data point, t/apache/read3.t fails for me consistently on the
>> kfreebsd-{i386,amd64} porter boxes (fischer and falla) but interestingly
>> not on the kfreebsd buildds.
>>
>> The test is getting an internal server error, with this in the
>> error log:
>>
>>   [Thu Aug 07 13:30:28.768239 2014] [perl:error] [pid 81995:tid 34593212160] [client 127.0.0.1:12832] Apache2::RequestIO::read: (70007) The timeout specified has expired at /home/ntyni/libapache2-mod-perl2-2.0.8+httpd24-r1449661/t/response/TestApache/read3.pm line 30
>
> As this was fully reproducible on the Debian/kFreeBSD boxes, I spent
> some time debugging it.
>
[...]
>
> I'm unsure if there's a fault with the client (it gets an FD that's
> ready for writing from the first select() call but ignores it, calls
> select() again and blocks because the server has already called poll()
> on the same descriptor), but the deadlock can be fixed/worked around by
> making the server not send a response before the full POST request has
> been read. See the attached patch, which fixes this completely for me.
>
> I can also reproduce the issue on amd64 by running the test in a loop and
> putting some load on the host. The patch makes it go away there too.
>
> Cc'ing the modperl dev list. Please consider applying the patch.

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?

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.