Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(177192)

#23377: HTTPResponse may drop buffer holding next response

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 5 months ago by vadmium+py
Modified:
4 years ago
Reviewers:
demianbrecht
CC:
AntoinePitrou, Martin Panter, demian
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 25

Patch Set 3 #

Total comments: 3

Patch Set 4 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/http.client.rst View 1 2 3 4 chunks +13 lines, -2 lines 0 comments Download
Lib/http/client.py View 1 2 3 16 chunks +57 lines, -41 lines 0 comments Download
Lib/test/test_httplib.py View 1 2 3 8 chunks +79 lines, -11 lines 0 comments Download
Lib/test/test_urllib2.py View 1 2 3 2 chunks +2 lines, -3 lines 0 comments Download
Lib/test/test_urllib.py View 1 2 3 1 chunk +13 lines, -17 lines 0 comments Download
Lib/urllib/request.py View 1 2 3 3 chunks +4 lines, -15 lines 0 comments Download

Messages

Total messages: 8
demian
http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (left): http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#oldcode400 Lib/http/client.py:400: def flush(self): Why remove this? I think that if ...
4 years, 1 month ago #1
Martin Panter
http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (left): http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#oldcode400 Lib/http/client.py:400: def flush(self): On 2015/05/21 02:02:36, demian wrote: > Why ...
4 years, 1 month ago #2
demian
http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (left): http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#oldcode400 Lib/http/client.py:400: def flush(self): On 2015/05/21 08:13:39, vadmium wrote: > On ...
4 years, 1 month ago #3
Martin Panter
http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (right): http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#newcode220 Lib/http/client.py:220: self.fp = fp On 2015/05/21 21:50:45, demian wrote: > ...
4 years, 1 month ago #4
demian
http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (right): http://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#newcode220 Lib/http/client.py:220: self.fp = fp On 2015/05/22 02:41:44, vadmium wrote: > ...
4 years, 1 month ago #5
Martin Panter
https://bugs.python.org/review/23377/diff/14480/Lib/http/client.py File Lib/http/client.py (right): https://bugs.python.org/review/23377/diff/14480/Lib/http/client.py#newcode1169 Lib/http/client.py:1169: if self._reader is None: On 2015/05/22 18:24:52, demian wrote: ...
4 years, 1 month ago #6
demian
http://bugs.python.org/review/23377/diff/14919/Lib/http/client.py File Lib/http/client.py (right): http://bugs.python.org/review/23377/diff/14919/Lib/http/client.py#newcode213 Lib/http/client.py:213: will_close=False): Should align with opening bracket on previous line. ...
4 years, 1 month ago #7
Martin Panter
4 years, 1 month ago #8
http://bugs.python.org/review/23377/diff/14919/Lib/http/client.py
File Lib/http/client.py (right):

http://bugs.python.org/review/23377/diff/14919/Lib/http/client.py#newcode838
Lib/http/client.py:838: reader = self._reader
On 2015/06/07 23:39:43, demian wrote:
> I feel like I'm missing something obvious, but what's the benefit of this flow
> over:
> 
> if self._reader:
>     self._reader.close()
>     self._reader = None

Now that you brought it up, I think this simpler _reader.close() version should
be fine. Other than an unlucky KeyboardInterrupt or programmer error (which can
never be 100% avoided), I don’t think it can raise an exception.

> Likewise for the socket related code below.

I agree it is not obvious; see <https://bugs.python.org/issue23865>. In this
case I suspect it was mainly a theoretical problem, because there are no write
buffers to flush. Maybe overriding odd methods in a subclass or messing with
socket settings, etc, could cause a close() call to fail.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+