From 3ac667fac0c7a571d60198919360dee284419f3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Tue, 25 Oct 2016 12:20:14 +0200 Subject: [PATCH 1/5] Add failing test that triggers #155 --- integrationtests.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/integrationtests.py b/integrationtests.py index 7d06ff2a..9a3194b4 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -18,6 +18,8 @@ import sys import tempfile import unittest +import time +import concurrent.futures import clcache @@ -1071,6 +1073,41 @@ def testHitsViaMpConcurrent(self): self.assertEqual(stats.numCacheMisses(), 2) self.assertEqual(stats.numCacheEntries(), 2) +class TestConcurrent(unittest.TestCase): + @staticmethod + def runCompiler(): + with cd(os.path.join(ASSETS_DIR, "hits-and-misses")): + cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c", 'hit.cpp'] + return subprocess.check_call(cmd) + + def testDoesntCrashWhenTwoProcessesAddAnObject(self): + # object cache key + cachekey = 'b0f233141cfe8ec8bf7af259591e3777' + + # Obtain final object lock and write the object file + cache = clcache.Cache() + section = cache.compilerArtifactsRepository.section(cachekey) + section.lock.acquire() + + executor = concurrent.futures.ThreadPoolExecutor(max_workers=1) + compilerTask = executor.submit(TestConcurrent.runCompiler) + + # Give the clcache process time to get locked acquiring the artifactsSection lock + time.sleep(1) + + # Replacing artifacts + print('Replacing artifacts') + artifactsPath = lambda objectName: os.path.join(section.cacheEntryDir(cachekey), objectName) + artifacts = [artifactsPath('object'), artifactsPath('output.txt'), artifactsPath('stderr.txt')] + clcache.ensureDirectoryExists(section.cacheEntryDir(cachekey)) + for artifact in artifacts: + with open(artifact, 'w+') as f: + f.write('dummy') + + section.lock.release() + + timeoutSeconds = 5 + self.assertEqual(0, compilerTask.result(timeout=timeoutSeconds)) class TestBasedir(unittest.TestCase): def testBasedir(self): From adf808a4aa87c6ce1ac0a00a3d2c6c9da96ae716 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Tue, 25 Oct 2016 13:44:43 +0200 Subject: [PATCH 2/5] Rename method (pylint) and add documentation --- integrationtests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/integrationtests.py b/integrationtests.py index 9a3194b4..f7702beb 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -1080,7 +1080,10 @@ def runCompiler(): cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c", 'hit.cpp'] return subprocess.check_call(cmd) - def testDoesntCrashWhenTwoProcessesAddAnObject(self): + def testRaceAddingAnObject(self): + """ This tests attempts to expose a race condition when two clcache invocations decide to add an object to the + cache at the same time """ + # object cache key cachekey = 'b0f233141cfe8ec8bf7af259591e3777' From 7bcc8abd91e4d3f1a8034e359b1ec0d8066ea468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Tue, 25 Oct 2016 14:34:26 +0200 Subject: [PATCH 3/5] Increase sleep time to trigger race condition --- integrationtests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrationtests.py b/integrationtests.py index f7702beb..504fec2b 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -1096,7 +1096,7 @@ def testRaceAddingAnObject(self): compilerTask = executor.submit(TestConcurrent.runCompiler) # Give the clcache process time to get locked acquiring the artifactsSection lock - time.sleep(1) + time.sleep(5) # Replacing artifacts print('Replacing artifacts') From 684d54935efd2da96b5f75089a693a1aab17f602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Tue, 25 Oct 2016 15:02:31 +0200 Subject: [PATCH 4/5] Force crlf attribute on text files to have reproducible object hashes --- .gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..92be83e2 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text eol=crlf From a932706791ffad0c11dd9a0672e9f74e6fa9b23d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Tue, 25 Oct 2016 16:09:33 +0200 Subject: [PATCH 5/5] Update cachekey --- integrationtests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrationtests.py b/integrationtests.py index 504fec2b..850277ed 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -1085,7 +1085,7 @@ def testRaceAddingAnObject(self): cache at the same time """ # object cache key - cachekey = 'b0f233141cfe8ec8bf7af259591e3777' + cachekey = '5080f03ed6fda8fecd7562c0fad99926' # Obtain final object lock and write the object file cache = clcache.Cache()