From 9ef0413aebe105b9aa5b122b80c41acf406b6a3a Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 16 Nov 2021 16:05:56 +0100 Subject: [PATCH 1/4] atomically create the CACHE_TAG file, see #6028 --- src/borg/helpers/fs.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index 2ec90a0b7..066687d59 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -10,6 +10,7 @@ import textwrap from .errors import Error from .process import prepare_subprocess_env +from ..platform import SaveFile from ..platformflags import is_win32 from ..constants import * # NOQA @@ -92,15 +93,14 @@ def get_cache_dir(): cache_dir = os.environ.get('BORG_CACHE_DIR', os.path.join(cache_home, 'borg')) # Create path if it doesn't exist yet ensure_dir(cache_dir) - cache_fn = os.path.join(cache_dir, CACHE_TAG_NAME) - if not os.path.exists(cache_fn): - with open(cache_fn, 'wb') as fd: - fd.write(CACHE_TAG_CONTENTS) - fd.write(textwrap.dedent(""" - # This file is a cache directory tag created by Borg. - # For information about cache directory tags, see: - # http://www.bford.info/cachedir/spec.html - """).encode('ascii')) + cache_tag_fn = os.path.join(cache_dir, CACHE_TAG_NAME) + cache_tag_contents = CACHE_TAG_CONTENTS + textwrap.dedent(""" + # This file is a cache directory tag created by Borg. + # For information about cache directory tags, see: + # http://www.bford.info/cachedir/spec.html + """).encode('ascii') + with SaveFile(cache_tag_fn, binary=True) as fd: + fd.write(cache_tag_contents) return cache_dir From 7f5cd2c4d471410fa6f64f1fd2261e3b9845897f Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 17 Nov 2021 17:36:43 +0100 Subject: [PATCH 2/4] fix cyclic import issues --- src/borg/helpers/fs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index 066687d59..f448ba654 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -10,7 +10,6 @@ import textwrap from .errors import Error from .process import prepare_subprocess_env -from ..platform import SaveFile from ..platformflags import is_win32 from ..constants import * # NOQA @@ -99,6 +98,7 @@ def get_cache_dir(): # For information about cache directory tags, see: # http://www.bford.info/cachedir/spec.html """).encode('ascii') + from ..platform import SaveFile with SaveFile(cache_tag_fn, binary=True) as fd: fd.write(cache_tag_contents) return cache_dir From d0a3b30fdf2db48fd9079b4507cd6369fc08fe2b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 7 Dec 2021 21:47:15 +0100 Subject: [PATCH 3/4] avoid create the cache tag file on every get_cache_dir call this re-introduces a race between os.path.exists vs. SaveFile creating that file, but due to the way how SaveFile works, it still makes sure that in the end there is a good cache tag file in place. --- src/borg/helpers/fs.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index f448ba654..3bd89e2f2 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -93,14 +93,15 @@ def get_cache_dir(): # Create path if it doesn't exist yet ensure_dir(cache_dir) cache_tag_fn = os.path.join(cache_dir, CACHE_TAG_NAME) - cache_tag_contents = CACHE_TAG_CONTENTS + textwrap.dedent(""" - # This file is a cache directory tag created by Borg. - # For information about cache directory tags, see: - # http://www.bford.info/cachedir/spec.html - """).encode('ascii') - from ..platform import SaveFile - with SaveFile(cache_tag_fn, binary=True) as fd: - fd.write(cache_tag_contents) + if not os.path.exists(cache_tag_fn): + cache_tag_contents = CACHE_TAG_CONTENTS + textwrap.dedent(""" + # This file is a cache directory tag created by Borg. + # For information about cache directory tags, see: + # http://www.bford.info/cachedir/spec.html + """).encode('ascii') + from ..platform import SaveFile + with SaveFile(cache_tag_fn, binary=True) as fd: + fd.write(cache_tag_contents) return cache_dir From 708a5853e7410f24ac385fbf74d751d7e017720b Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 7 Dec 2021 22:35:28 +0100 Subject: [PATCH 4/4] deal with the SaveFile/SyncFile race, docs --- src/borg/helpers/fs.py | 10 ++++++++-- src/borg/platform/base.py | 9 ++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/borg/helpers/fs.py b/src/borg/helpers/fs.py index 3bd89e2f2..d52ce5f12 100644 --- a/src/borg/helpers/fs.py +++ b/src/borg/helpers/fs.py @@ -100,8 +100,14 @@ def get_cache_dir(): # http://www.bford.info/cachedir/spec.html """).encode('ascii') from ..platform import SaveFile - with SaveFile(cache_tag_fn, binary=True) as fd: - fd.write(cache_tag_contents) + try: + with SaveFile(cache_tag_fn, binary=True) as fd: + fd.write(cache_tag_contents) + except FileExistsError: + # if we have multiple SaveFile calls running in parallel for same cache_tag_fn, + # it is fine if just one (usually first/quicker one) of them run gets through + # and all others raise FileExistsError. + pass return cache_dir diff --git a/src/borg/platform/base.py b/src/borg/platform/base.py index ba94f91d0..c6351b24b 100644 --- a/src/borg/platform/base.py +++ b/src/borg/platform/base.py @@ -141,12 +141,14 @@ class SyncFile: Note that POSIX doesn't specify *anything* about power failures (or similar failures). A system that routinely loses files or corrupts file on power loss is POSIX compliant. + Calling SyncFile(path) for an existing path will raise FileExistsError, see comment in __init__. + TODO: Use F_FULLSYNC on OSX. TODO: A Windows implementation should use CreateFile with FILE_FLAG_WRITE_THROUGH. """ def __init__(self, path, binary=False): - mode = 'xb' if binary else 'x' + mode = 'xb' if binary else 'x' # x -> raise FileExists exception in open() if file exists already self.fd = open(path, mode) self.fileno = self.fd.fileno() @@ -193,6 +195,11 @@ class SaveFile: On a journaling file system the file contents are always updated atomically and won't become corrupted, even on power failures or crashes (for caveats see SyncFile). + + Calling SaveFile(path) in parallel for same path is safe (even when using the same SUFFIX), but the + caller needs to catch potential FileExistsError exceptions that may happen in this racy situation. + The caller executing SaveFile->SyncFile->open() first will win. + All other callers will raise a FileExistsError in open(), at least until the os.replace is executed. """ SUFFIX = '.tmp'