( )⚙ D3246 keepalive: rewrite readinto() to not use read()

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3246

keepalive: rewrite readinto() to not use read()
ClosedPublic

Authored by durin42 on Apr 11 2018, 3:50 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG192b7ad06932: keepalive: rewrite readinto() to not use read()
Summary

It turns out http.client on Python 3 sometimes uses readinto() in the
implementation of read(). Unfortunately, Python 2 doesn't have a
readinto() in httplib's client, so we have to support both codepaths.

Subclassing is bad, folks.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Apr 11 2018, 3:50 PM
durin42 edited the summary of this revision. (Show Details)Apr 11 2018, 4:42 PM
durin42 updated this revision to Diff 8015.
indygreg accepted this revision.Apr 12 2018, 11:15 AM
indygreg added a subscriber: indygreg.

This is horrible. But this is the world we live in. Everything about the HTTP code terrifies me. That includes both our code and CPython's.

This revision is now accepted and ready to land.Apr 12 2018, 11:15 AM
Closed by commit rHG192b7ad06932: keepalive: rewrite readinto() to not use read() (authored by durin42). · Explain WhyApr 12 2018, 11:34 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/keepalive.py (25 lines)

Diff 8065

mercurial/keepalive.py

self._rbuf = '' self._rbuf = ''
self._rbufsize = 8096 self._rbufsize = 8096
self._handler = None # inserted by the handler later self._handler = None # inserted by the handler later
self._host = None # (same) self._host = None # (same)
self._url = None # (same) self._url = None # (same)
self._connection = None # (same) self._connection = None # (same)
_raw_read = httplib.HTTPResponse.read _raw_read = httplib.HTTPResponse.read
_raw_readinto = getattr(httplib.HTTPResponse, 'readinto', None)
def close(self): def close(self):
if self.fp: if self.fp:
self.fp.close() self.fp.close()
self.fp = None self.fp = None
if self._handler: if self._handler:
self._handler._request_closed(self, self._host, self._handler._request_closed(self, self._host,
self._connection) self._connection)
break break
list.append(line) list.append(line)
total += len(line) total += len(line)
if sizehint and total >= sizehint: if sizehint and total >= sizehint:
break break
return list return list
def readinto(self, dest): def readinto(self, dest):
if self._raw_readinto is None:
res = self.read(len(dest)) res = self.read(len(dest))
if not res: if not res:
return 0 return 0
dest[0:len(res)] = res dest[0:len(res)] = res
return len(res) return len(res)
total = len(dest)
have = len(self._rbuf)
if have >= total:
dest[0:total] = self._rbuf[:total]
self._rbuf = self._rbuf[total:]
return total
mv = memoryview(dest)
got = self._raw_readinto(mv[have:total])
dest[0:have] = self._rbuf
got += len(self._rbuf)
self._rbuf = ''
return got
def safesend(self, str): def safesend(self, str):
"""Send `str' to the server. """Send `str' to the server.
Shamelessly ripped off from httplib to patch a bad behavior. Shamelessly ripped off from httplib to patch a bad behavior.
""" """
# _broken_pipe_resp is an attribute we set in this function # _broken_pipe_resp is an attribute we set in this function
# if the socket is closed while we're sending data but # if the socket is closed while we're sending data but