From 262e789fca6950bc136a87f9a475d6cf7dd1faa3 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Wed, 3 Oct 2018 21:56:22 +0900 Subject: utils.py: Give save_file_atomic() a tempdir argument Allow callers to decide where the temporary file will be created. --- buildstream/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/buildstream/utils.py b/buildstream/utils.py index efecdc2e5..1bb504470 100644 --- a/buildstream/utils.py +++ b/buildstream/utils.py @@ -496,7 +496,7 @@ def get_bst_version(): @contextmanager def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None, - errors=None, newline=None, closefd=True, opener=None): + errors=None, newline=None, closefd=True, opener=None, tempdir=None): """Save a file with a temporary name and rename it into place when ready. This is a context manager which is meant for saving data to files. @@ -523,8 +523,9 @@ def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None, # https://bugs.python.org/issue8604 assert os.path.isabs(filename), "The utils.save_file_atomic() parameter ``filename`` must be an absolute path" - dirname = os.path.dirname(filename) - fd, tempname = tempfile.mkstemp(dir=dirname) + if tempdir is None: + tempdir = os.path.dirname(filename) + fd, tempname = tempfile.mkstemp(dir=tempdir) os.close(fd) f = open(tempname, mode=mode, buffering=buffering, encoding=encoding, -- cgit v1.2.1 From 10d6998819c75e8be8d01bec6c98a666b6ee4e08 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Wed, 3 Oct 2018 21:57:43 +0900 Subject: _artifactcache/cascache.py: Don't create temporary files in the CAS storage Use the designated tempdir when creating refs, we expect that temporary files are not created in the storage directory ever, they should be only ever created in the designated temporary directory. This fixes race conditions where utils._get_dir_size() throws an unhandled exception when attempting to stat the file which inadvertantly disappears. --- buildstream/_artifactcache/cascache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildstream/_artifactcache/cascache.py b/buildstream/_artifactcache/cascache.py index 14932fba2..ffeccc0b3 100644 --- a/buildstream/_artifactcache/cascache.py +++ b/buildstream/_artifactcache/cascache.py @@ -418,7 +418,7 @@ class CASCache(ArtifactCache): def set_ref(self, ref, tree): refpath = self._refpath(ref) os.makedirs(os.path.dirname(refpath), exist_ok=True) - with utils.save_file_atomic(refpath, 'wb') as f: + with utils.save_file_atomic(refpath, 'wb', tempdir=self.tmpdir) as f: f.write(tree.SerializeToString()) # resolve_ref(): -- cgit v1.2.1 From c02a1ae86bb2dacf0f2b9f1e9254516304b32731 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Wed, 3 Oct 2018 22:00:22 +0900 Subject: utils.py: Document _get_dir_size() expectations. This function assumes that files do not disappear while walking the given directory. --- buildstream/utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/buildstream/utils.py b/buildstream/utils.py index 1bb504470..34c0f898b 100644 --- a/buildstream/utils.py +++ b/buildstream/utils.py @@ -557,6 +557,9 @@ def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None, # # Get the disk usage of a given directory in bytes. # +# This function assumes that files do not inadvertantly +# disappear while this function is running. +# # Arguments: # (str) The path whose size to check. # -- cgit v1.2.1