Control: tag -1 patch

On Sat, Dec 15, 2018 at 09:53:54PM +0200, Niko Tyni wrote:
> On Sat, Dec 15, 2018 at 03:52:58AM +0100, gregor herrmann wrote:
> > Package: autopkgtest
> > Version: 5.7
> > Severity: normal
> 
> > autopkgtest [03:29:08]: ERROR: "chmod -R go+rwX -- 
> > /tmp/autopkgtest.ES3bLl/autopkgtest-satdep.deb" failed with stderr 
> > "/bin/chmod: cannot access 
> > '/tmp/autopkgtest.ES3bLl/autopkgtest-satdep.deb': No such file or directory
 
> It looks to me like a race where the TempPath destructor for
> autopkgtest-satdep.deb gets run after the next test has created a new
> TempPath object for the same filename.

I see a few different ways to fix this.

A. Find out why the TempPath.__del__ finalizer gets called late

   I suppose there must be a reference cycle involved but I can't see
   it offhand. This seems to be enough of a heisenbug that just 'import gc'
   makes it go away for my test case, making debugging hard.

B. Remove the TempPath.__del__ finalizer and add explicit cleanup calls

   It looks like there are only two uses with autoclean enabled in the
   code, so this seems doable.

C. Make TempPath use unique file names so collisions don't matter

   It would probably be best to do this only when autoclean is enabled,
   so that anything saved in the output dir (= autoclean disabled)
   would keep its current name.

The attached patch implements C as a minimal change.
-- 
Niko
>From 8c39a1dd0942e265ab48ce5b7a418b7ce435d451 Mon Sep 17 00:00:00 2001
From: Niko Tyni <nt...@debian.org>
Date: Sun, 16 Dec 2018 20:10:54 +0200
Subject: [PATCH] Ensure that autocleaned TempPath file names are unique

In some circumstances the TempPath.__del__() finalizer may get
run late enough that a new TempPath instance has been created
with the same name.

Prepend a running counter to the actual file names used to prevent
collisions.

Closes: #916499
---
 lib/adt_testbed.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/adt_testbed.py b/lib/adt_testbed.py
index f46a4df..4fe8874 100644
--- a/lib/adt_testbed.py
+++ b/lib/adt_testbed.py
@@ -1484,6 +1484,10 @@ class TempPath(Path):
 
     These are only guaranteed to exit within one testbed run.
     '''
+
+    # private; used to make sure the names of autocleaned files don't collide
+    _filename_prefix = 1
+
     def __init__(self, testbed, name, is_dir=False, autoclean=True):
         '''Create a temporary Path object.
 
@@ -1507,6 +1511,10 @@ class TempPath(Path):
         else:
             host = testbed.output_dir
         self.autoclean = autoclean
+        if autoclean:
+            name = str(self._filename_prefix) + '-' + name
+            TempPath._filename_prefix += 1
+
         Path.__init__(self, testbed, os.path.join(host, name),
                       os.path.join(testbed.scratch, name),
                       is_dir)
-- 
2.20.0

Reply via email to