This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: httplib closes socket, then tries to read from it
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.0
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: janssen Nosy List: BreamoreBoy, Rhamphoryncus, georg.brandl, gvanrossum, janssen, jhylton, martin.panter, vila
Priority: normal Keywords: patch

Created on 2007-10-28 00:49 by janssen, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
c janssen, 2007-10-28 01:19
unnamed janssen, 2007-11-27 20:19
Messages (13)
msg56870 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-10-28 00:49
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.
msg56871 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-10-28 01:19
I believe this is all that's needed.
msg56878 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-10-28 12:59
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.
msg56916 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-10-29 17:44
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.
msg57740 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-21 22:30
Is this still relevant?
msg57745 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-11-21 23:08
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>
> __________________________________
>
msg57827 - (view) Author: (vila) Date: 2007-11-25 11:09
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.
msg57851 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-11-26 19:42
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>
> __________________________________
>
msg57853 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-26 21:11
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...
msg57872 - (view) Author: (vila) Date: 2007-11-27 14:21
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...
msg57883 - (view) Author: Bill Janssen (janssen) * (Python committer) Date: 2007-11-27 20:19
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>
> __________________________________
>
msg114602 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-08-21 23:10
Is this still relevant?
msg116783 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-09-18 14:22
No reply to msg114602 so I'll close in a couple of weeks unless anyone objects.
History
Date User Action Args
2022-04-11 14:56:27adminsetgithub: 45689
2013-11-08 07:31:06martin.pantersetnosy: + martin.panter
2013-11-06 19:50:36akuchlingsetstatus: pending -> closed
resolution: not a bug
2010-09-18 14:22:36BreamoreBoysetstatus: open -> pending
nosy: + BreamoreBoy
messages: + msg116783

2010-08-21 23:10:10georg.brandlsetnosy: + georg.brandl
messages: + msg114602
2008-05-10 20:16:20jhyltonsetnosy: + jhylton
2008-05-07 17:51:11Rhamphoryncussetnosy: + Rhamphoryncus
2008-01-06 22:29:45adminsetkeywords: - py3k
versions: Python 3.0
2007-11-27 20:19:14janssensetfiles: + unnamed
messages: + msg57883
2007-11-27 14:21:58vilasetmessages: + msg57872
2007-11-26 21:11:27gvanrossumsetmessages: + msg57853
2007-11-26 19:42:59janssensetmessages: + msg57851
2007-11-25 11:09:12vilasetnosy: + vila
messages: + msg57827
2007-11-21 23:08:06janssensetmessages: + msg57745
2007-11-21 22:30:33gvanrossumsetpriority: critical -> normal
assignee: janssen
messages: + msg57740
2007-10-29 17:44:29gvanrossumsetnosy: + gvanrossum
messages: + msg56916
2007-10-28 12:59:45janssensetkeywords: + patch
messages: + msg56878
2007-10-28 01:19:25janssensetfiles: + c
messages: + msg56871
2007-10-28 00:49:13janssencreate