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

login
register
mail settings
Submitter phabricator
Date Jan. 21, 2020, 6:37 p.m.
Message ID <differential-rev-PHID-DREV-ckebprztkitdbfmtbzbl-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44554/
State Superseded
Headers show

Comments

phabricator - Jan. 21, 2020, 6:37 p.m.
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




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

Patch

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']: