[PATCH] fix hang because of infinite loop reading EOF

[Sebastian Spaeth]

On Thu, 03 Mar 2011 10:46:33 +0800, Haojun Bao <baohaojun at gmail.com> wrote:
> Offlineimap sometimes hangs when downloading gmail with large
> attachment.
> 
> This is because of several infinite loop when reading the socket while
> the EOF has happened. It can be seen from 2 facts:

Hi,

I think your patch might be safe and should most probably be included
(with some comment in the code explaining why we need that check and
abort I guess). However, will that be enough? If we are in the state you
describe the connection has basically been closed and the file
descriptor is worthless, right?

So it is ok to abort the read(), but it will not close the connection,
and we will be trying to read from the same connection soon again. And
subsequent read() attempts will also not be successful then, right?

So what we should do in a case where we detect a broken connection is to
properly kill off the connection and re-establish it.

Or am I completely wrong here?

> After reading the mailing list, I see Gábor Melis has proposed a patch
> (SO_KEEPALIVE) to fix hang, I don't know if my case is same as his. But
> anyway, I think the several read methods contain bugs in their
> implementation.

THe SO_KEEPALIVE will certainly help to prevent some connection closes,
but that is basically not related to the fact that python's read() on
SSL objects doesn't seem to fail when the connection has been closed
already, so I think you might be right about some bugs looming in the
ssl read() functionality. Again, I am not sure that just aborting the
read() loop will be enough to fix the hangs, as we won't close the
connection and try to use the same ssl socket to read soon enough
again. At least that is what I would suspect without having checked. 

> You can use the following patch to fix the problem. 
> (I have posted it on the news server, but not sure did it get through.)

I am all for including these, if there is a nice commit message
or code comment, explaining why this is basically needed.

Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/offlineimap-project/attachments/20110303/9bfb93ed/attachment-0001.sig>