D7957: lfs: explicitly close the file handle for the blob being uploaded

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Tue Jan 21 18:37:48 UTC 2020


mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The previous code relied on reading the blob fully to close it.  The obvious
  problem is if an error occurs before that point.  But there is also a problem
  when using workers where the data may need to be re-read, which can't happen
  once it is closed.  This eliminates the surprising behavior before attempting to
  fix the worker problem.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7957

AFFECTED FILES
  hgext/lfs/blobstore.py

CHANGE DETAILS

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -110,11 +110,12 @@
     def read(self, size):
         if self._fp is None:
             return b''
-        data = self._fp.read(size)
-        if not data:
+        return self._fp.read(size)
+
+    def close(self):
+        if self._fp is not None:
             self._fp.close()
             self._fp = None
-        return data
 
 
 class local(object):
@@ -539,6 +540,9 @@
             raise LfsRemoteError(
                 _(b'LFS error: %s') % _urlerrorreason(ex), hint=hint
             )
+        finally:
+            if request.data:
+                request.data.close()
 
     def _batch(self, pointers, localstore, action):
         if action not in [b'upload', b'download']:



To: mharbison72, #hg-reviewers
Cc: mercurial-devel


More information about the Mercurial-devel mailing list