Fix lock race condition by unlinking before closing the file handle.

1 files changed, 11 insertions, 11 deletions

diff --git a/lib/common/NamedLock.cpp b/lib/common/NamedLock.cpp
index da309d73..c862bb86 100644
--- a/lib/common/NamedLock.cpp
+++ b/lib/common/NamedLock.cpp
@@ -207,29 +207,29 @@ void NamedLock::ReleaseLock()
 	{
 		THROW_EXCEPTION(CommonException, NamedLockNotHeld)
 	}
-	
-	// Close the file
-	if(::close(mFileDescriptor) != 0)
+
+	// Delete the file. We need to do this before closing the filehandle, otherwise
+	// someone could acquire the lock, release and delete it between us closing (and
+	// hence releasing) and deleting it, and we'd fail when it came to deleting the
+	// file. This happens in tests much more often than you'd expect!
+
+	if(::unlink(mFileName.c_str()) != 0)
 	{
 		THROW_EMU_ERROR(
-			BOX_FILE_MESSAGE(mFileName, "Failed to close lockfile"),
+			BOX_FILE_MESSAGE(mFileName, "Failed to delete lockfile"),
 			CommonException, OSFileError);
 	}
 
-	// Delete the file
-	if(::unlink(mFileName.c_str()) != 0)
+	// Close the file
+	if(::close(mFileDescriptor) != 0)
 	{
 		THROW_EMU_ERROR(
-			BOX_FILE_MESSAGE(mFileName,
-				"Failed to delete lockfile"),
+			BOX_FILE_MESSAGE(mFileName, "Failed to close lockfile"),
 			CommonException, OSFileError);
 	}
 
 	// Mark as unlocked, so we don't try to close it again if the unlink() fails.
 	mFileDescriptor = -1;
-}
-
-
 
 	BOX_TRACE("Released lock and deleted lockfile " << mFileName);
 }