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

Revision history for this message
Robert Collins (lifeless) wrote :

On Wed, 2009-07-08 at 00:09 +0000, Martin Pool wrote:

> I can imagine some deadlocks occurring because they're both in the
> same process - perhaps the socket is relying on gc collect and it's
> not running before the deadlock occurs.

Thats another possibility yes. Good catch.

> > > Would you prefer KnownFailures for pycurl+https instead ?
> >
> > I'd prefer to know the real cause. If valid http behaviour makes
> pycurl
> > hang, then real servers on the internet will be able to make bzr
> hang,
> > and thats bad. As it stands, I feel that landing this is a bit of an
> > anti-fix, as it is a workaround we can't reasonably ask live
> internet
> > servers to make.
>
> I don't think we should hold this patch hostage to that. It's making
> the test server more correct and it's fixing a real problem that the
> test suite hangs. If we get reports that bzr hangs against real
> http/1.1 servers, which I don't remember seeing so far, then we can
> look at how to test that.

The patch *does not* make the server more correct. It does prevent
hangs, but still, I think we should not stop looking at this till we
know the actual cause. Otherwise we've only half completed the root
cause analysis.

One reason we probably don't see reports of hangs, is that error cases
are relatively rare for bzr - we generally request resources that exist.

-Rob

« Back to merge proposal