From f098b44a1a5efe50ee6fc1fd5c67470837c342b3 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Tue, 2 Apr 2019 13:54:06 -0700 Subject: [PATCH] cmake: atomic rename to fix toolchain cache creation race While this race seems unlikely and harmless let's play it safe and implement the usual solution: temporary file + atomic rename. The race is unlikely and maybe even harmless because: - Only sanitycheck seems to invoke cmake concurrently. - Users rarely delete their ~/.cache/zephyr/ToolchainCapabilityDatabase/ - All concurrent cmake processes write the same, single byte to the same files. - Creating a single byte is at least very fast, so extremely short window for others to read an empty file. For additional background see links in issue #9992 Signed-off-by: Marc Herbert --- cmake/extensions.cmake | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/cmake/extensions.cmake b/cmake/extensions.cmake index 58d9fe3e4a8787..fcba086fb7b691 100644 --- a/cmake/extensions.cmake +++ b/cmake/extensions.cmake @@ -679,11 +679,31 @@ function(zephyr_check_compiler_flag lang option check) # Populate the cache if(NOT (EXISTS ${key_path})) + + # This is racy. As often with race conditions, this one can easily be + # made worse and demonstrated with a simple delay: + # execute_process(COMMAND "sleep" "5") + # Delete the cache, add the sleep above and run sanitycheck with a + # large number of JOBS. Once it's done look at the log.txt file + # below and you will see that concurrent cmake processes created the + # same files multiple times. + + # While there are a number of reasons why this race seems both very + # unlikely and harmless, let's play it safe anyway and write to a + # private, temporary file first. All modern filesystems seem to + # support at least one atomic rename API and cmake's file(RENAME + # ...) officially leverages that. + string(RANDOM LENGTH 8 tempsuffix) + file( WRITE - ${key_path} + "${key_path}_tmp_${tempsuffix}" ${inner_check} ) + file( + RENAME + "${key_path}_tmp_${tempsuffix}" "${key_path}" + ) # Populate a metadata file (only intended for trouble shooting) # with information about the hash, the toolchain capability