[PATCH 06 of 10] explicitly close files

Dan Villiom Podlaski Christiansen danchr at gmail.com
Wed Dec 1 15:34:59 CST 2010


# HG changeset patch
# User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
# Date 1291236059 -3600
# Node ID 401824d53a179b0855256ef829f1bb5abfd29501
# Parent  ef3a4a0c604721a89f5f029f6ab3a3e1949928e2
explicitly close files

Add missing calls to close() to many places where files are
opened. Relying on reference counting to catch them soon-ish is not
portable and fails in environments with a proper GC, such as PyPy.

diff --git a/hgext/gpg.py b/hgext/gpg.py
--- a/hgext/gpg.py
+++ b/hgext/gpg.py
@@ -244,7 +244,9 @@ def sign(ui, repo, *revs, **opts):
                            "(please commit .hgsigs manually "
                            "or use --force)"))
 
-    repo.wfile(".hgsigs", "ab").write(sigmessage)
+    sigsfile = repo.wfile(".hgsigs", "ab")
+    sigsfile.write(sigmessage)
+    sigsfile.close()
 
     if '.hgsigs' not in repo.dirstate:
         repo[None].add([".hgsigs"])
diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -239,6 +239,7 @@ class queue(object):
         try:
             fh = open(os.path.join(path, 'patches.queue'))
             cur = fh.read().rstrip()
+            fh.close()
             if not cur:
                 curpath = os.path.join(path, 'patches')
             else:
@@ -1763,6 +1764,7 @@ class queue(object):
                 checkfile(patchname)
                 patchf = self.opener(patchname, "w")
                 patchf.write(text)
+                patchf.close()
             if not force:
                 checkseries(patchname)
             if patchname not in self.series:
@@ -2760,6 +2762,7 @@ def qqueue(ui, repo, name=None, **opts):
         try:
             fh = repo.opener(_allqueues, 'r')
             queues = [queue.strip() for queue in fh if queue.strip()]
+            fh.close()
             if current not in queues:
                 queues.append(current)
         except IOError:
diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -82,6 +82,7 @@ class tarit(object):
 
     def __init__(self, dest, mtime, kind=''):
         self.mtime = mtime
+        self.fileobj = None
 
         def taropen(name, mode, fileobj=None):
             if kind == 'gz':
@@ -91,8 +92,10 @@ class tarit(object):
                 gzfileobj = self.GzipFileWithTime(name, mode + 'b',
                                                   zlib.Z_BEST_COMPRESSION,
                                                   fileobj, timestamp=mtime)
+                self.fileobj = gzfileobj
                 return tarfile.TarFile.taropen(name, mode, gzfileobj)
             else:
+                self.fileobj = fileobj
                 return tarfile.open(name, mode + kind, fileobj)
 
         if isinstance(dest, str):
@@ -118,6 +121,8 @@ class tarit(object):
 
     def done(self):
         self.z.close()
+        if self.fileobj:
+            self.fileobj.close()
 
 class tellable(object):
     '''provide tell method for zipfile.ZipFile when writing to http
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -678,7 +678,8 @@ def export(repo, revs, template='hg-%h.p
             parents.reverse()
         prev = (parents and parents[0]) or nullid
 
-        if not fp:
+        localfp = not fp
+        if localfp:
             fp = make_file(repo, template, node, total=total, seqno=seqno,
                            revwidth=revwidth, mode='ab')
         if fp != sys.stdout and hasattr(fp, 'name'):
@@ -699,9 +700,13 @@ def export(repo, revs, template='hg-%h.p
         for chunk in patch.diff(repo, prev, node, opts=opts):
             fp.write(chunk)
 
+        if localfp:
+            fp.close()
+
     for seqno, rev in enumerate(revs):
         single(rev, seqno + 1, fp)
 
+
 def diffordiffstat(ui, repo, diffopts, node1, node2, match,
                    changes=None, stat=False, fp=None, prefix='',
                    listsubrepos=False):
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1019,7 +1019,9 @@ def debugcomplete(ui, cmd='', **opts):
 
 def debugfsinfo(ui, path = "."):
     """show information detected about current filesystem"""
-    open('.debugfsinfo', 'w').write('')
+    fp = open('.debugfsinfo', 'w')
+    fp.write('')
+    fp.close()
     ui.write('exec: %s\n' % (util.checkexec(path) and 'yes' or 'no'))
     ui.write('symlink: %s\n' % (util.checklink(path) and 'yes' or 'no'))
     ui.write('case-sensitive: %s\n' % (util.checkcase('.debugfsinfo')
@@ -1383,7 +1385,9 @@ def debuginstall(ui):
         if list(files) != [os.path.basename(fa)]:
             ui.write(_(" unexpected patch output!\n"))
             patchproblems += 1
-        a = open(fa).read()
+        fp = open(fa)
+        a = fp.read()
+        fp.close()
         if a != b:
             ui.write(_(" patch test failed!\n"))
             patchproblems += 1
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -80,7 +80,9 @@ class dirstate(object):
     @propertycache
     def _pl(self):
         try:
-            st = self._opener("dirstate").read(40)
+            fp = self._opener("dirstate")
+            st = fp.read(40)
+            fp.close()
             l = len(st)
             if l == 40:
                 return st[:20], st[20:40]
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -153,14 +153,18 @@ def share(ui, source, dest=None, update=
             raise
 
     requirements += 'shared\n'
-    file(os.path.join(roothg, 'requires'), 'w').write(requirements)
-    file(os.path.join(roothg, 'sharedpath'), 'w').write(sharedpath)
+
+    def write(fn, data):
+        fp = file(os.path.join(roothg, fn), 'w')
+        fp.write(data)
+        fp.close()
+
+    write('requires', requirements)
+    write('sharedpath', sharedpath)
 
     default = srcrepo.ui.config('paths', 'default')
     if default:
-        f = file(os.path.join(roothg, 'hgrc'), 'w')
-        f.write('[paths]\ndefault = %s\n' % default)
-        f.close()
+        write('hgrc', '[paths]\ndefault = %s\n' % default)
 
     r = repository(ui, root)
 
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -277,6 +277,8 @@ class localrepository(repo.repository):
         # committed tags are stored in UTF-8
         writetags(fp, names, encoding.fromlocal, prevtags)
 
+        fp.close()
+
         if '.hgtags' not in self.dirstate:
             self[None].add(['.hgtags'])
 
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -32,6 +32,7 @@ class mergestate(object):
                 else:
                     bits = l[:-1].split("\0")
                     self._state[bits[0]] = bits[1:]
+            f.close()
         except IOError, err:
             if err.errno != errno.ENOENT:
                 raise
@@ -42,6 +43,7 @@ class mergestate(object):
             f.write(hex(self._local) + "\n")
             for d, v in self._state.iteritems():
                 f.write("\0".join([d] + v) + "\n")
+            f.close()
             self._dirty = False
     def add(self, fcl, fco, fca, fd, flags):
         hash = util.sha1(fcl.path()).hexdigest()
@@ -67,6 +69,7 @@ class mergestate(object):
         state, hash, lfile, afile, anode, ofile, flags = self._state[dfile]
         f = self._repo.opener("merge/" + hash)
         self._repo.wwrite(dfile, f.read(), flags)
+        f.close()
         fcd = wctx[dfile]
         fco = octx[ofile]
         fca = self._repo.filectx(afile, fileid=anode)
diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -71,7 +71,9 @@ def set_flags(f, l, x):
     if l:
         if not stat.S_ISLNK(s):
             # switch file to link
-            data = open(f).read()
+            fp = open(f)
+            data = fp.read()
+            fp.close()
             os.unlink(f)
             try:
                 os.symlink(data, f)
@@ -84,7 +86,9 @@ def set_flags(f, l, x):
         # switch link to file
         data = os.readlink(f)
         os.unlink(f)
-        open(f, "w").write(data)
+        fp = open(f, "w")
+        fp.write(data)
+        fp.close()
         s = 0666 & ~umask # avoid restatting for chmod
 
     sx = s & 0100
diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -100,7 +100,9 @@ class statichttprepository(localrepo.loc
                 raise
             # check if it is a non-empty old-style repository
             try:
-                self.opener("00changelog.i").read(1)
+                fh = self.opener("00changelog.i")
+                fh.read(1)
+                fh.close()
             except IOError, inst:
                 if inst.errno != errno.ENOENT:
                     raise
diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -27,13 +27,17 @@ def _playback(journal, report, opener, e
     for f, o, ignore in entries:
         if o or not unlink:
             try:
-                opener(f, 'a').truncate(o)
+                fp = opener(f, 'a')
+                fp.truncate(o)
+                fp.close()
             except IOError:
                 report(_("failed to truncate %s\n") % f)
                 raise
         else:
             try:
-                fn = opener(f).name
+                fp = opener(f)
+                fn = fp.name
+                fp.close()
                 os.unlink(fn)
             except (IOError, OSError), inst:
                 if inst.errno != errno.ENOENT:
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -198,7 +198,10 @@ def tempfilter(s, cmd):
         if code:
             raise Abort(_("command '%s' failed: %s") %
                         (cmd, explain_exit(code)))
-        return open(outname, 'rb').read()
+        fp = open(outname, 'rb')
+        r = fp.read()
+        fp.close()
+        return r
     finally:
         try:
             if inname:
@@ -598,7 +601,10 @@ def readlock(pathname):
             raise
     except AttributeError: # no symlink in os
         pass
-    return posixfile(pathname).read()
+    fp = posixfile(pathname)
+    r = fp.read()
+    fp.close()
+    return r
 
 def fstat(fp):
     '''stat file object that may not have fileno method.'''


More information about the Mercurial-devel mailing list