urllib hangs when closing connection

[rg3]

BPO	11199
Nosy	@orsenthil, @giampaolo
Files	test.py: Minimal case that reproduces the problem urllib_ftp_close_27.diff: Patch without wait for 2.7 urllib_ftp_close.diff: Patch without wait for 3k

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-03-15.20:28:48.211>
created_at = <Date 2011-02-12.13:00:03.941>
labels = ['extension-modules', 'type-bug']
title = 'urllib hangs when closing connection'
updated_at = <Date 2012-03-15.20:28:48.209>
user = 'https://bugs.python.org/rg3'

bugs.python.org fields:

activity = <Date 2012-03-15.20:28:48.209>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2012-03-15.20:28:48.211>
closer = 'python-dev'
components = ['Extension Modules']
creation = <Date 2011-02-12.13:00:03.941>
creator = 'rg3'
dependencies = []
files = ['20750', '20847', '20848']
hgrepos = []
issue_num = 11199
keywords = ['patch']
message_count = 11.0
messages = ['128444', '128919', '128978', '128980', '128999', '129005', '129040', '129110', '129115', '129116', '155953']
nosy_count = 5.0
nosy_names = ['orsenthil', 'giampaolo.rodola', 'rg3', 'neologix', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue11199'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

[rg3]

If you run the attached program, you can see the program hangs in the connection close stage. Uncommenting the sleep line makes the program work, so I suspect some kind of race condition.

The URL used belongs to a Slackware Linux mirror. I have not been able to reproduce this problem when using a different FTP mirror or using HTTP mirrors.

The remote server seems to be using pure-ftpd. I have built the software and tested on localhost, but I could not reproduce the problem either.

[neologix]

The problem is due to the way urllib closes a FTP data transfer.
The data channel is closed, and a shutdown hook is called that waits for a message on the control channel. But in that case, when the data connection is closed while the transfer is in progress, the server doesn't send any further message on the control channel, and we remain stuck (note that if the data channel is closed after the file has been transfered, in that case an end of transfer message is sent, which explains why this dones't happen with the sleep in between).
The solution is to first wait for a message on the control channel, and then close the data channel (which makes sense, a close hook is generally called before closing the corresponding connection).
The attached patch just does that.
Note that I'm not sure why we need to wait for a further message on the control channel (maybe it's part of an RFC or something...).

[rg3]

That makes sense and explains why the problem could not be reproduced over the loopback (the transfer would be too fast).

I have not tested the patch, but I can reproduce the problem with a local connection if I compile pure-ftpd with the --with-throttling switch and limit the bandwidth to 1 KB/sec, using the -t option.

[rg3]

I just tested the patch under Python 2.6. It doesn't seem to solve the problem.

[neologix]

I just tested the patch under Python 2.6. It doesn't seem to solve the problem.

Are you sure the patch applied cleanly ?
I tested both on 3.2 and 2.7, and it fixed the problem for me.
If not, could you submit a tcpdump capture ?

[rg3]

I have to correct myself. I applied the patch manually to my Python 2.6 installation. In Python 2.6, the line you moved is number 961, and I did the same change.

With your change, the connection can be closed, but you have to wait for the file to be completely transferred. As I was throttling to 1 KB/sec initially, I thought it was still hanging because it takes more than 1 minute for the test file to be sent. Still, the connection isn't immediately closed when you request to close it.

[neologix]

rg3 <sarbalap+freshmeat@gmail.com> added the comment:

I have to correct myself. I applied the patch manually to my Python 2.6 installation. In Python 2.6, the line you moved is number 961, and I did the same change.

OK. For information, you can apply it using the Unix "patch" command,
who can most of the time do all the work for you.

With your change, the connection can be closed, but you have to wait for the file to be completely transferred. As I was throttling to 1 KB/sec initially, I thought it was still hanging because it takes more than 1 minute for the test file to be sent. Still, the connection isn't immediately closed when you request to close it.

That's expected, it's a consequence of this point I raised earlier:

Note that I'm not sure why we need to wait for a further message on the control channel (maybe it's part of an RFC or something...).

The current code explicitely waits for the end of transfer before
closing the data channel.
Don't ask me why, I don't have a clue. I wrote a 2-line patch to
disable this behaviour which seems to work fine, but since I'm not
sure why the code is doing this right now, I'd like some feedback
before doing the change.

[rg3]

> I have to correct myself. I applied the patch manually to my Python
> 2.6 installation. In Python 2.6, the line you moved is number 961,
> and I did the same change.

OK. For information, you can apply it using the Unix "patch" command,
who can most of the time do all the work for you.

Yes, thanks, I already knew about "patch" but decided to apply the
change manually just in case, as it belonged to a different Python
branch.

That's expected, it's a consequence of this point I raised earlier:
> Note that I'm not sure why we need to wait for a further message on
> the control channel (maybe it's part of an RFC or something...).

There's no doubt that not hanging is an improvement, but the current
behavior somewhat defeats the purpose of an early call to "close".

To get a broader view, the test case I provided is actually part of a
much larger program. In that program, I read lines from the Slackware
change log, stopping when I read a line that the program already knows
about (because it caches the change log contents). This prevents the
program from reading more than a single line if no new entries are
present in the change log, but having to wait for the full file defeats
the purpose, specially on slow dial-up connections.

[neologix]

Attached are two new versions which don't wait for the end of transfer.

[rg3]

Charles-Francois Natali, Tuesday, February 22, 2011 20:57:

Attached are two new versions which don't wait for the end of
transfer.

I tested the one for Python 2.7 applied to Python 2.6 and it appears to
work perfectly. Thanks for the quick fix!

[python-dev]

New changeset 6ce4868861ba by Senthil Kumaran in branch '2.7':
Fix the urllib closing issue which hangs on particular ftp urls/ftp servers. closes bpo-11199
http://hg.python.org/cpython/rev/6ce4868861ba

New changeset 891184abbf6e by Senthil Kumaran in branch '3.2':
closes Issue bpo-11199: Fix the with urllib which hangs on particular ftp urls.
http://hg.python.org/cpython/rev/891184abbf6e

New changeset 9e7374779e19 by Senthil Kumaran in branch 'default':
port from 3.2 - Fix the urllib closing issue which hangs on particular ftp urls/ftp servers. closes bpo-11199
http://hg.python.org/cpython/rev/9e7374779e19