Rev 3204: Fix two more leaking tmp dirs, by reworking TransformPreview lock handling. in file:///v/home/vila/src/bzr/bugs/123363-tmp-pollution/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jan 29 15:16:37 GMT 2008


At file:///v/home/vila/src/bzr/bugs/123363-tmp-pollution/

------------------------------------------------------------
revno: 3204
revision-id:v.ladeuil+lp at free.fr-20080129151631-vqjd13tb405mobx6
parent: v.ladeuil+lp at free.fr-20080129132736-nai2jpv02r2wltgc
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 123363-tmp-pollution
timestamp: Tue 2008-01-29 16:16:31 +0100
message:
  Fix two more leaking tmp dirs, by reworking TransformPreview lock handling.
  
  * bzrlib/tests/test_transform.py:
  (TestTransformMerge): Revert previous patch and cleanly call
  preview.finalize now that we can.
  
  * bzrlib/tests/test_merge.py:
  (TestMerge.test_make_preview_transform): Catch TransformPreview
  leak.
  
  * bzrlib/builtins.py:
  (cmd_merge._do_preview): Finalize the TransformPreview or the
  limbodir will stay in /tmp.
  
  * bzrlib/transform.py:
  (TreeTransformBase.__init__): Create the _deletiondir since it's
  reffered to by finalize.
  (TreeTransformBase.finalize): Delete the dir only if _deletiondir
  is set.
  (TreeTransform.__init__): Use a temp var for deletiondir and set
  the attribute after the base class __init__ has been called.
  (TransformPreview.__init__): Read locks the tree since finalize
  wants to unlock it (as suggested by Aaron).
modified:
  bzrlib/builtins.py             builtins.py-20050830033751-fc01482b9ca23183
  bzrlib/tests/test_merge.py     testmerge.py-20050905070950-c1b5aa49ff911024
  bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
-------------- next part --------------
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py	2008-01-23 16:25:18 +0000
+++ b/bzrlib/builtins.py	2008-01-29 15:16:31 +0000
@@ -2877,6 +2877,7 @@
         result_tree = tt.get_preview_tree()
         show_diff_trees(merger.this_tree, result_tree, self.outf, old_label='',
                         new_label='')
+        tt.finalize()
 
     def _do_merge(self, merger, change_reporter, allow_pending, verified):
         merger.change_reporter = change_reporter

=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py	2008-01-05 23:01:50 +0000
+++ b/bzrlib/tests/test_merge.py	2008-01-29 15:16:31 +0000
@@ -385,6 +385,7 @@
         merger.merge_type = _mod_merge.Merge3Merger
         tree_merger = merger.make_merger()
         tt = tree_merger.make_preview_transform()
+        self.addCleanup(tt.finalize)
         preview_tree = tt.get_preview_tree()
         tree_file = this_tree.get_file('file-id')
         try:

=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2008-01-29 13:27:36 +0000
+++ b/bzrlib/tests/test_transform.py	2008-01-29 15:16:31 +0000
@@ -1233,6 +1233,7 @@
 
 
 class TestTransformMerge(TestCaseInTempDir):
+
     def test_text_merge(self):
         root_id = generate_ids.gen_root_id()
         base = TransformGroup("base", root_id)
@@ -1834,30 +1835,24 @@
         repository = self.make_repository('repo')
         tree = repository.revision_tree(_mod_revision.NULL_REVISION)
         preview = TransformPreview(tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         return preview
 
-    def addPreviewCleanup(self, preview):
-        # This probably indicates a problem in the TransformPreview
-        # API. finalize() can't be called in tests unsing addPreviewCleanup
-        # because no lock is held.
-        self.addCleanup(lambda: osutils.rmtree(preview._limbodir))
-
     def test_transform_preview(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
 
     def test_transform_preview_tree(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         preview.get_preview_tree()
 
     def test_transform_new_file(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         preview.new_file('file2', preview.root, 'content B\n', 'file2-id')
         preview_tree = preview.get_preview_tree()
         self.assertEqual(preview_tree.kind('file2-id'), 'file')
@@ -1867,7 +1862,7 @@
     def test_diff_preview_tree(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         preview.new_file('file2', preview.root, 'content B\n', 'file2-id')
         preview_tree = preview.get_preview_tree()
         out = StringIO()
@@ -1880,7 +1875,7 @@
     def test_transform_conflicts(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         preview.new_file('a', preview.root, 'content 2')
         resolve_conflicts(preview)
         trans_id = preview.trans_id_file_id('a-id')
@@ -1889,7 +1884,7 @@
     def get_tree_and_preview_tree(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         a_trans_id = preview.trans_id_file_id('a-id')
         preview.delete_contents(a_trans_id)
         preview.create_file('b content', a_trans_id)
@@ -1947,7 +1942,7 @@
     def test_kind(self):
         revision_tree = self.create_tree()
         preview = TransformPreview(revision_tree)
-        self.addPreviewCleanup(preview)
+        self.addCleanup(preview.finalize)
         preview.new_file('file', preview.root, 'contents', 'file-id')
         preview.new_directory('directory', preview.root, 'dir-id')
         preview_tree = preview.get_preview_tree()

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2008-01-29 13:27:36 +0000
+++ b/bzrlib/transform.py	2008-01-29 15:16:31 +0000
@@ -81,6 +81,7 @@
         object.__init__(self)
         self._tree = tree
         self._limbodir = limbodir
+        self._deletiondir = None
         self._id_number = 0
         # mapping of trans_id -> new basename
         self._new_name = {}
@@ -158,7 +159,8 @@
                 # We don't especially care *why* the dir is immortal.
                 raise ImmortalLimbo(self._limbodir)
             try:
-                os.rmdir(self._deletiondir)
+                if self._deletiondir is not None:
+                    os.rmdir(self._deletiondir)
             except OSError:
                 raise errors.ImmortalPendingDeletion(self._deletiondir)
         finally:
@@ -1136,19 +1138,20 @@
             except OSError, e:
                 if e.errno == errno.EEXIST:
                     raise ExistingLimbo(limbodir)
-            self._deletiondir = urlutils.local_path_from_url(
+            deletiondir = urlutils.local_path_from_url(
                 control_files.controlfilename('pending-deletion'))
             try:
-                os.mkdir(self._deletiondir)
+                os.mkdir(deletiondir)
             except OSError, e:
                 if e.errno == errno.EEXIST:
-                    raise errors.ExistingPendingDeletion(self._deletiondir)
+                    raise errors.ExistingPendingDeletion(deletiondir)
         except:
             tree.unlock()
             raise
 
         TreeTransformBase.__init__(self, tree, limbodir, pb,
                                    tree.case_sensitive)
+        self._deletiondir = deletiondir
 
     def apply(self, no_conflicts=False, _mover=None):
         """Apply all changes to the inventory and filesystem.
@@ -1321,6 +1324,7 @@
     """
 
     def __init__(self, tree, pb=DummyProgress(), case_sensitive=True):
+        tree.lock_read()
         limbodir = tempfile.mkdtemp(prefix='bzr-limbo-')
         TreeTransformBase.__init__(self, tree, limbodir, pb, case_sensitive)
 



More information about the bazaar-commits mailing list