[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 <bmr@redhat.com>

1 files changed, 12 insertions, 2 deletions

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')