Code review comment for lp:~vila/bzr/383920-pycurl-hangs

>>>>> "robert" == Robert Collins <email address hidden> writes:

    robert> On Tue, 2009-07-07 at 13:15 +0000, Vincent Ladeuil wrote:
    >> Right. So the test server was:
    >> - presenting itself as 1.1,
    >> - not sending the Content-Length,
    >> - sending 'Connection: close',
    >> - closing the connection

    robert> This is valid. How were we closing it? shutdown(both),

No

    robert> or halfshutdown,

Not sure what you mean.

    robert> then close, or ...?

Close the read and write parts of the socket. Simple closes.

    robert> Perhaps this question doesn't matter, if doing
    robert> exactly the same thing but adding the header was
    robert> enough to stop the symptoms.

I was about to either:
- look at the C code,
- test against real servers (via my local_test_server) plugins,
- keep thinking...

And then I thought about the send_error method that I didn't
fully trust in the past and thought, well, let's just try
that.. and it worked.

    >> and the client hanged, so following your arguments, it
    >> means the *client* is broken, right ? Or is there still
    >> some grey area because the server close on error while
    >> announcing 1.1 ?

    robert> Is it using https, or just http?

https *only*, http is fine...

    robert> I can imagine that there is some https teardown stuff
    robert> that perhaps we aren't doing right.

Whatever we do, a network connection loss will not, the client is
not supposed to hang either in that case no ?

    >> Would you prefer KnownFailures for pycurl+https instead ?

    robert> I'd prefer to know the real cause.

Cost/benefit analysis stopped me there, whatever the bug is, it
came from a change in either python or pycurl as I can guarantee
that at one point I had a python2.6/https/pycurl combo passing
the full test suite. It's a pity I didn't take note of the
precise versions involved.

    robert> If valid http behaviour makes pycurl hang, then real
    robert> servers on the internet will be able to make bzr
    robert> hang, and thats bad.

I totally share that view.

    robert> As it stands, I feel that landing this is a bit of an
    robert> anti-fix, as it is a workaround we can't reasonably
    robert> ask live internet servers to make.

Iff there are servers in the wild acting as described above (1.1,
close on error without Content-Length)...

My plan for that is to keep adding real servers to the
local_test_server plugin.

Overall, I think my time is better spent that way than fixing
bugs in pycurl instead of working on certificate validation for
urllib...

        Vincent