<div dir="ltr">a question here: if this was a race, it could have been found with tsan (this or a similar one *was* caught by tsan). <div>Do we have enough tests for multi-threaded code to run a tsan bootstrap bot? </div><div>
<br></div><div>--kcc </div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Mar 6, 2014 at 9:37 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:akyrtzi@gmail.com" target="_blank">akyrtzi@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: akirtzidis<br>
Date: Thu Mar  6 11:37:10 2014<br>
New Revision: 203138<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=203138&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=203138&view=rev</a><br>
Log:<br>
[Support/LockFileManager] Make the LockFileManager more robust against races.<br>
<br>
There was a race where:<br>
- The LockFileManager tries to own the lock file and fails.<br>
- The other owner then releases and removes the lock file.<br>
- The LockFileManager tries to read the owner info from the lock file but fails now.<br>
<br>
In such a case have LockFileManager try to get ownership again, instead of error'ing out.<br>
<br>
Modified:<br>
    llvm/trunk/lib/Support/LockFileManager.cpp<br>
<br>
Modified: llvm/trunk/lib/Support/LockFileManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=203138&r1=203137&r2=203138&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/LockFileManager.cpp?rev=203138&r1=203137&r2=203138&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/lib/Support/LockFileManager.cpp (original)<br>
+++ llvm/trunk/lib/Support/LockFileManager.cpp Thu Mar  6 11:37:10 2014<br>
@@ -115,25 +115,42 @@ LockFileManager::LockFileManager(StringR<br>
     }<br>
   }<br>
<br>
-  // Create a symbolic link from the lock file name. If this succeeds, we're done.<br>
-  // Note that we are using symbolic link because hard links are not supported<br>
-  // by all filesystems.<br>
-  error_code EC<br>
-    = sys::fs::create_symbolic_link(UniqueLockFileName.str(),<br>
-                                      LockFileName.str());<br>
-  if (EC == errc::success)<br>
-    return;<br>
-<br>
-  // Someone else managed to create the lock file first. Wipe out our unique<br>
-  // lock file (it's useless now) and read the process ID from the lock file.<br>
-  sys::fs::remove(UniqueLockFileName.str());<br>
-  if ((Owner = readLockFile(LockFileName)))<br>
-    return;<br>
-<br>
-  // There is a lock file that nobody owns; try to clean it up and report<br>
-  // an error.<br>
-  sys::fs::remove(LockFileName.str());<br>
-  Error = EC;<br>
+  while (1) {<br>
+    // Create a symbolic link from the lock file name. If this succeeds, we're<br>
+    // done. Note that we are using symbolic link because hard links are not<br>
+    // supported by all filesystems.<br>
+    error_code EC<br>
+      = sys::fs::create_symbolic_link(UniqueLockFileName.str(),<br>
+                                        LockFileName.str());<br>
+    if (EC == errc::success)<br>
+      return;<br>
+<br>
+    if (EC != errc::file_exists) {<br>
+      Error = EC;<br>
+      return;<br>
+    }<br>
+<br>
+    // Someone else managed to create the lock file first. Read the process ID<br>
+    // from the lock file.<br>
+    if ((Owner = readLockFile(LockFileName))) {<br>
+      // Wipe out our unique lock file (it's useless now)<br>
+      sys::fs::remove(UniqueLockFileName.str());<br>
+      return;<br>
+    }<br>
+<br>
+    if (!sys::fs::exists(LockFileName.str())) {<br>
+      // The previous owner released the lock file before we could read it.<br>
+      // Try to get ownership again.<br>
+      continue;<br>
+    }<br>
+<br>
+    // There is a lock file that nobody owns; try to clean it up and get<br>
+    // ownership.<br>
+    if ((EC = sys::fs::remove(LockFileName.str()))) {<br>
+      Error = EC;<br>
+      return;<br>
+    }<br>
+  }<br>
 }<br>
<br>
 LockFileManager::LockFileState LockFileManager::getState() const {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>