[clang][Dependency Scanning] Move Module Timestamp Update After Compilation Finishes

[qiongsiwu]

When two threads are accessing the same pcm, it is possible that the reading thread sees the timestamp update, while the file on disk is not updated.

This PR moves timestamp update from writeAST to compileModuleAndReadASTImpl, so we only update the timestamp after the file has been committed to disk.

rdar://152097193

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qiongsi Wu (qiongsiwu)

Changes

When two threads are accessing the same pcm, it is possible that the reading thread sees the timestamp update, while the file on disk is not updated.

This PR moves timestamp update from writeAST to compileModuleAndReadASTImpl, so we only update the timestamp when the file is committed to disk.

rdar://152097193

---

Full diff: https://github.com/llvm/llvm-project/pull/151774.diff

2 Files Affected:

• (modified) clang/lib/Frontend/CompilerInstance.cpp (+8)
• (modified) clang/lib/Serialization/ASTWriter.cpp (-5)

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index ed6a651d919a1..ae285b117f5e0 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1480,6 +1480,14 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance,
     return false;
   }
 
+  // The module is built successfully, we can update its timestamp now.
+  if (ImportingInstance.getPreprocessor()
+          .getHeaderSearchInfo()
+          .getHeaderSearchOpts()
+          .ModulesValidateOncePerBuildSession) {
+    ImportingInstance.getModuleCache().updateModuleTimestamp(ModuleFileName);
+  }
+
   return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
                                    Module, ModuleFileName,
                                    /*OutOfDate=*/nullptr, /*Missing=*/nullptr);
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index a6957e54b66f1..0c7ddd07553a4 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5461,11 +5461,6 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject,
 
   WritingAST = false;
 
-  if (WritingModule && PPRef.getHeaderSearchInfo()
-                           .getHeaderSearchOpts()
-                           .ModulesValidateOncePerBuildSession)
-    ModCache.updateModuleTimestamp(OutputFile);
-
   if (ShouldCacheASTInMemory) {
     // Construct MemoryBuffer and update buffer manager.
     ModCache.getInMemoryModuleCache().addBuiltPCM(

[llvmbot]

@llvm/pr-subscribers-clang-modules

Author: Qiongsi Wu (qiongsiwu)

Changes

When two threads are accessing the same pcm, it is possible that the reading thread sees the timestamp update, while the file on disk is not updated.

This PR moves timestamp update from writeAST to compileModuleAndReadASTImpl, so we only update the timestamp when the file is committed to disk.

rdar://152097193

---

Full diff: https://github.com/llvm/llvm-project/pull/151774.diff

2 Files Affected:

• (modified) clang/lib/Frontend/CompilerInstance.cpp (+8)
• (modified) clang/lib/Serialization/ASTWriter.cpp (-5)

diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index ed6a651d919a1..ae285b117f5e0 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1480,6 +1480,14 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance,
     return false;
   }
 
+  // The module is built successfully, we can update its timestamp now.
+  if (ImportingInstance.getPreprocessor()
+          .getHeaderSearchInfo()
+          .getHeaderSearchOpts()
+          .ModulesValidateOncePerBuildSession) {
+    ImportingInstance.getModuleCache().updateModuleTimestamp(ModuleFileName);
+  }
+
   return readASTAfterCompileModule(ImportingInstance, ImportLoc, ModuleNameLoc,
                                    Module, ModuleFileName,
                                    /*OutOfDate=*/nullptr, /*Missing=*/nullptr);
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index a6957e54b66f1..0c7ddd07553a4 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5461,11 +5461,6 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject,
 
   WritingAST = false;
 
-  if (WritingModule && PPRef.getHeaderSearchInfo()
-                           .getHeaderSearchOpts()
-                           .ModulesValidateOncePerBuildSession)
-    ModCache.updateModuleTimestamp(OutputFile);
-
   if (ShouldCacheASTInMemory) {
     // Construct MemoryBuffer and update buffer manager.
     ModCache.getInMemoryModuleCache().addBuiltPCM(