httplib closes socket, then tries to read from it

[janssen]

BPO	1348
Nosy	@gvanrossum, @birkenfeld, @vadmium
Files	c unnamed

^{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 2013-11-06.19:50:36.100>
created_at = <Date 2007-10-28.00:49:13.098>
labels = ['invalid', 'type-bug', 'library']
title = 'httplib closes socket, then tries to read from it'
updated_at = <Date 2013-11-08.07:31:06.929>
user = 'https://bugs.python.org/janssen'

bugs.python.org fields:

activity = <Date 2013-11-08.07:31:06.929>
actor = 'martin.panter'
assignee = 'janssen'
closed = True
closed_date = <Date 2013-11-06.19:50:36.100>
closer = 'akuchling'
components = ['Library (Lib)']
creation = <Date 2007-10-28.00:49:13.098>
creator = 'janssen'
dependencies = []
files = ['8638', '8814']
hgrepos = []
issue_num = 1348
keywords = ['patch']
message_count = 13.0
messages = ['56870', '56871', '56878', '56916', '57740', '57745', '57827', '57851', '57853', '57872', '57883', '114602', '116783']
nosy_count = 8.0
nosy_names = ['gvanrossum', 'jhylton', 'georg.brandl', 'janssen', 'Rhamphoryncus', 'vila', 'BreamoreBoy', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1348'
versions = ['Python 3.0']

[janssen]

I can't get urllib.urlopen() to work with SSL, and it seems to be due to
a bug in the re-write of httplib. HTTPConnection.getresponse() is
closing the socket, but then returning a response object holding onto
that closed socket to the caller, who then tries to read from the
(closed) socket. Due to the delayed closing of sockets, this error is
masked, but in SSL, the context is torn down on close, so we see real
failures.

[janssen]

I believe this is all that's needed.

[janssen]

I still think the semantics are wrong here, but someone needs to close
this connection at some point, and right now the reference-counting
semantics of socket.close() are the only thing preventing a leak. So I
think my patch should not be applied. Instead, a larger fix needs to be
thought through, which will probably require some changes to urllib and
its cousins.

[gvanrossum]

I don't think that patch is the right thing either. Certainly the
comment on the line you're proposing to delete is suggesting that the
close() call was added for a reason.

[gvanrossum]

Is this still relevant?

[janssen]

Yes. The close is stil in the wrong place.

On Nov 21, 2007 2:30 PM, Guido van Rossum <report@bugs.python.org> wrote:

Guido van Rossum added the comment:

Is this still relevant?

----------
assignee: -> janssen
priority: urgent -> normal

---

Tracker <report@bugs.python.org>
<http://bugs.python.org/issue1348\>

---

[vila]

The title of this bug is scary.

httplib rightly close the socket because that's the way to transfer the
responsibility of the close to the user of the HttpResponse object.

The close MUST stays there.

I do encounter a bug related to that close while trying to use the ssl
module in python2.6.

I'm willing to help fixing it but *this* bug may not be the right place
to do so (even if related).

I'm a bit lost on how to help right now since the involved changes seems
to occur in both python3.0 and python2.6 and I'm currently targeting 2.4
and 2.5.

[janssen]

No, the close must be removed. It's the wrong *way* to "transfer
responsibility". Close means "close", not "I wash my hands of
responsibility here". What's hidden here is that the open socket is
transferred to the response, which continues to use it. In fact, the
close() should happen when the caller is finished with the response,
probably as effect of the GC of the "response" object.

On Nov 25, 2007 3:09 AM, vila <report@bugs.python.org> wrote:

vila added the comment:

The title of this bug is scary.

httplib rightly close the socket because that's the way to transfer the
responsibility of the close to the user of the HttpResponse object.

The close MUST stays there.

I do encounter a bug related to that close while trying to use the ssl
module in python2.6.

I'm willing to help fixing it but *this* bug may not be the right place
to do so (even if related).

I'm a bit lost on how to help right now since the involved changes seems
to occur in both python3.0 and python2.6 and I'm currently targeting 2.4
and 2.5.

----------
nosy: +vila

---

Tracker <report@bugs.python.org>
<http://bugs.python.org/issue1348\>

---

[gvanrossum]

Bill, is there a code example that should work but breaks because of
that close()? ATM, there doesn't seem to be anything in the tests that
breaks...

[vila]

Well I was confused.

In fact, I have a working http/1.1 client which indeed works around that
close.

HTTPConnection.close() must be called once the response has been
processed or the next request can't be handled since
HTTPConnection.__state is not _CS_IDLE.

That may be a different problem than the one Bill is after though.

I work around it by saving the sock attribute before the call in a class
inherited from HTTPConnection:

         # Preserve our preciousss
        sock = self.sock
        self.sock = None
        # Let httplib.HTTPConnection do its housekeeping 
        self.close()
        # Restore our preciousss
        self.sock = sock

So not doing the close() is harmless but doing self.sock = None in
HTTPConnection.close() still break hopes of persistent connections.

May be that method should be splitted...

[janssen]

That's because the socket.py code has been adapted (the first word I wrote
there was "perverted" :--) to deal with this case. That is, the close() has
been rendered meaningless because of the explicit reference counting in
socket.py. But the right solution is to not close the socket till the
application is done with it; that is, transfer the responsibility for the
socket to the part of the application which is still using it. I'm not sure
that just fixing this one case will remove the need for the explicit
reference counting in socket.py, but this is the case that I noticed.

On Nov 26, 2007 1:11 PM, Guido van Rossum <report@bugs.python.org> wrote:

Guido van Rossum added the comment:

Bill, is there a code example that should work but breaks because of
that close()? ATM, there doesn't seem to be anything in the tests that
breaks...

---

Tracker <report@bugs.python.org>
<http://bugs.python.org/issue1348\>

---

[birkenfeld]

Is this still relevant?

[BreamoreBoy]

No reply to msg114602 so I'll close in a couple of weeks unless anyone objects.