From 7d098d89e509af9cbd3748bbae0fd3e1cf4df771 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Thu, 14 Jun 2018 15:37:19 +0100 Subject: [archive] make FileCacheArchive methods re-entrant Since plugins are now run concurrently the Archive class must be re-entrant: different plugins may attempt to create the same paths in the archive, and since they are executing in parallel and with no locking, there exists a TOCTOU race between the use of os.path.{exists,isdir, etc.}() and the subsequent creation of a new path. Address this by implementing a simple monitor-style locking scheme for the FileCacheArchive class. A `_path_lock` member is added to the class and this must be held across any operation that modifies the namespace of the archive (including file, directory, symlink, and special node creation). Additionally, the `_check_path()` Archive method (which checks for the existence of path components up to the basename of the file, creating directories as required) is extended to also carry out a test for the existence of the path, and if the path does exist, that the object is of the expected type. A mismatch in the object type generates a ValueError with an appropriate string description of the problem. Benchmarking shows a consistent small improvement with the patch: this is a result of reduced redundant copy operations that the previous archive structure allowed (since duplicate operations are now aborted as soon as the _path_lock is acquired). Resolves: #1340 Signed-off-by: Bryn M. Reeves --- tests/archive_tests.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/archive_tests.py b/tests/archive_tests.py index febc96b4..e3ee469c 100644 --- a/tests/archive_tests.py +++ b/tests/archive_tests.py @@ -92,12 +92,22 @@ class TarFileArchiveTest(unittest.TestCase): afp = self.tf.open_file('tests/string_test.txt') self.assertEquals('this is my content', afp.read()) - def test_overwrite_file(self): + def test_rewrite_file(self): + """Test that re-writing a file does not modify the content. + + In sos we do not have a use for overwriting archive content + in-place (it is an error if different plugins attempt to + store different content at the same path). + + We do not enforce the content check at runtime since it + would be prohibitively costly: instead just verify in the + unit suite that the original content is preserved. + """ self.tf.add_string('this is my content', 'tests/string_test.txt') self.tf.add_string('this is my new content', 'tests/string_test.txt') afp = self.tf.open_file('tests/string_test.txt') - self.assertEquals('this is my new content', afp.read()) + self.assertEquals('this is my content', afp.read()) def test_make_link(self): self.tf.add_file('tests/ziptest') -- cgit