Add non-blocking support to LoggingLockFile.

Bug: crashpad: 31
Change-Id: I67689eb1cb97c1feefe51e355a1509d97d2a0735
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2871970
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

client/settings.cc

@@ -207,9 +207,12 @@ Settings::ScopedLockedFileHandle Settings::MakeScopedLockedFileHandle(
   return ScopedLockedFileHandle(scoped.release(), base::FilePath());
 #else
   if (scoped.is_valid()) {
-    if (!LoggingLockFile(scoped.get(), locking))
+    if (LoggingLockFile(
+            scoped.get(), locking, FileLockingBlocking::kBlocking) !=
+        FileLockingResult::kSuccess) {
       scoped.reset();
     }
+  }
   return ScopedLockedFileHandle(scoped.release());
 #endif
 }

util/file/file_io.h

@@ -96,6 +96,28 @@ enum class FileLocking : bool {
   kExclusive,
 };
 
+//! \brief Determines if LoggingLockFile will block.
+enum class FileLockingBlocking : bool {
+  //! \brief Block until the lock is acquired
+  kBlocking = false,
+
+  //! \brief Do not block when attempting to acquire a lock.
+  kNonBlocking = true,
+};
+
+//! \brief The return value for LoggingLockFile.
+enum class FileLockingResult : int {
+  //! \brief The lock was acquired successfully.
+  kSuccess,
+
+  //! \brief In non-blocking mode only, the file was already locked. Locking
+  //!     would block, so the lock was not acquired.
+  kWouldBlock,
+
+  //! \brief The lock was not acquired.
+  kFailure,
+};
+
 //! \brief Determines the FileHandle that StdioFileHandle() returns.
 enum class StdioStream {
   //! \brief Standard input, or `stdin`.
@@ -443,8 +465,9 @@ FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
 //!     Windows.
 //!
 //! It is an error to attempt to lock a file in a different mode when it is
-//! already locked. This call will block until the lock is acquired. The
-//! entire file is locked.
+//! already locked. This call will block until the lock is acquired unless
+//! \a blocking is FileLockingBlocking::kNonBlocking. The entire file is
+//! locked.
 //!
 //! If \a locking is FileLocking::kShared, \a file must have been opened for
 //! reading, and if it's FileLocking::kExclusive, \a file must have been opened
@@ -453,9 +476,16 @@ FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
 //! \param[in] file The open file handle to be locked.
 //! \param[in] locking Controls whether the lock is a shared reader lock, or an
 //!     exclusive writer lock.
+//! \param[in] blocking Controls whether a locked file will result in blocking
+//!     or an immediate return.
 //!
-//! \return `true` on success, or `false` and a message will be logged.
-bool LoggingLockFile(FileHandle file, FileLocking locking);
+//! \return kSuccess if a lock is acquired. If a lock could not be acquired
+//!     because \a blocking is FileLockingBlocking::kNonBlocking and acquiring
+//!     the lock would block, returns kWouldBlock. If a lock fails for any other
+//!     reason, returns kFailure and a message will be logged.
+FileLockingResult LoggingLockFile(FileHandle file,
+                                  FileLocking locking,
+                                  FileLockingBlocking blocking);
 
 //! \brief Unlocks a file previously locked with LoggingLockFile().
 //!

util/file/file_io_posix.cc

@@ -210,11 +210,22 @@ FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
 
 #if !defined(OS_FUCHSIA)
 
-bool LoggingLockFile(FileHandle file, FileLocking locking) {
+FileLockingResult LoggingLockFile(FileHandle file,
+                                  FileLocking locking,
+                                  FileLockingBlocking blocking) {
   int operation = (locking == FileLocking::kShared) ? LOCK_SH : LOCK_EX;
+  if (blocking == FileLockingBlocking::kNonBlocking)
+    operation |= LOCK_NB;
+
   int rv = HANDLE_EINTR(flock(file, operation));
-  PLOG_IF(ERROR, rv != 0) << "flock";
-  return rv == 0;
+  if (rv != 0) {
+    if (errno == EWOULDBLOCK) {
+      return FileLockingResult::kWouldBlock;
+    }
+    PLOG(ERROR) << "flock";
+    return FileLockingResult::kFailure;
+  }
+  return FileLockingResult::kSuccess;
 }
 
 bool LoggingUnlockFile(FileHandle file) {

util/file/file_io_test.cc

@@ -561,11 +561,17 @@ TEST(FileIO, MultipleSharedLocks) {
 
   auto handle1 = ScopedFileHandle(LoggingOpenFileForRead(shared_file));
   ASSERT_NE(handle1, kInvalidFileHandle);
-  EXPECT_TRUE(LoggingLockFile(handle1.get(), FileLocking::kShared));
+  EXPECT_EQ(
+      LoggingLockFile(
+          handle1.get(), FileLocking::kShared, FileLockingBlocking::kBlocking),
+      FileLockingResult::kSuccess);
 
   auto handle2 = ScopedFileHandle(LoggingOpenFileForRead(shared_file));
   ASSERT_NE(handle1, kInvalidFileHandle);
-  EXPECT_TRUE(LoggingLockFile(handle2.get(), FileLocking::kShared));
+  EXPECT_EQ(
+      LoggingLockFile(
+          handle2.get(), FileLocking::kShared, FileLockingBlocking::kBlocking),
+      FileLockingResult::kSuccess);
 
   EXPECT_TRUE(LoggingUnlockFile(handle1.get()));
   EXPECT_TRUE(LoggingUnlockFile(handle2.get()));
@@ -590,7 +596,9 @@ class LockingTestThread : public Thread {
  private:
   void ThreadMain() override {
     for (int i = 0; i < iterations_; ++i) {
-      EXPECT_TRUE(LoggingLockFile(file_.get(), lock_type_));
+      EXPECT_EQ(LoggingLockFile(
+                    file_.get(), lock_type_, FileLockingBlocking::kBlocking),
+                FileLockingResult::kSuccess);
       base::subtle::NoBarrier_AtomicIncrement(actual_iterations_, 1);
       EXPECT_TRUE(LoggingUnlockFile(file_.get()));
     }
@@ -624,7 +632,9 @@ void LockingTest(FileLocking main_lock, FileLocking other_locks) {
                                     FileWriteMode::kReuseOrCreate,
                                     FilePermissions::kOwnerOnly));
   ASSERT_NE(initial, kInvalidFileHandle);
-  ASSERT_TRUE(LoggingLockFile(initial.get(), main_lock));
+  EXPECT_EQ(
+      LoggingLockFile(initial.get(), main_lock, FileLockingBlocking::kBlocking),
+      FileLockingResult::kSuccess);
 
   base::subtle::Atomic32 actual_iterations = 0;
 
@@ -671,6 +681,43 @@ TEST(FileIO, SharedVsExclusives) {
   LockingTest(FileLocking::kShared, FileLocking::kExclusive);
 }
 
+TEST(FileIO, ExclusiveVsExclusivesNonBlocking) {
+  ScopedTempDir temp_dir;
+  base::FilePath exclusive_file =
+      temp_dir.path().Append(FILE_PATH_LITERAL("file_to_lock"));
+
+  {
+    // Create an empty file to lock.
+    ScopedFileHandle create(
+        LoggingOpenFileForWrite(exclusive_file,
+                                FileWriteMode::kCreateOrFail,
+                                FilePermissions::kOwnerOnly));
+  }
+
+  auto handle1 = ScopedFileHandle(LoggingOpenFileForRead(exclusive_file));
+  ASSERT_NE(handle1, kInvalidFileHandle);
+  EXPECT_EQ(LoggingLockFile(handle1.get(),
+                            FileLocking::kExclusive,
+                            FileLockingBlocking::kBlocking),
+            FileLockingResult::kSuccess);
+
+  // Non-blocking lock should fail.
+  auto handle2 = ScopedFileHandle(LoggingOpenFileForRead(exclusive_file));
+  ASSERT_NE(handle2, kInvalidFileHandle);
+  EXPECT_EQ(LoggingLockFile(handle2.get(),
+                            FileLocking::kExclusive,
+                            FileLockingBlocking::kNonBlocking),
+            FileLockingResult::kWouldBlock);
+
+  // After unlocking, non-blocking lock should succeed.
+  EXPECT_TRUE(LoggingUnlockFile(handle1.get()));
+  EXPECT_EQ(LoggingLockFile(handle2.get(),
+                            FileLocking::kExclusive,
+                            FileLockingBlocking::kNonBlocking),
+            FileLockingResult::kSuccess);
+  EXPECT_TRUE(LoggingUnlockFile(handle2.get()));
+}
+
 #endif  // !OS_FUCHSIA
 
 TEST(FileIO, FileSizeByHandle) {

util/file/file_io_win.cc

@@ -182,19 +182,26 @@ FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
   return file;
 }
 
-bool LoggingLockFile(FileHandle file, FileLocking locking) {
+FileLockingResult LoggingLockFile(FileHandle file,
+                                  FileLocking locking,
+                                  FileLockingBlocking blocking) {
   DWORD flags =
       (locking == FileLocking::kExclusive) ? LOCKFILE_EXCLUSIVE_LOCK : 0;
+  if (blocking == FileLockingBlocking::kNonBlocking)
+    flags |= LOCKFILE_FAIL_IMMEDIATELY;
 
   // Note that the `Offset` fields of overlapped indicate the start location for
   // locking (beginning of file in this case), and `hEvent` must be also be set
   // to 0.
   OVERLAPPED overlapped = {0};
   if (!LockFileEx(file, flags, 0, MAXDWORD, MAXDWORD, &overlapped)) {
-    PLOG(ERROR) << "LockFileEx";
-    return false;
+    if (GetLastError() == ERROR_LOCK_VIOLATION) {
+      return FileLockingResult::kWouldBlock;
     }
-  return true;
+    PLOG(ERROR) << "LockFileEx";
+    return FileLockingResult::kFailure;
+  }
+  return FileLockingResult::kSuccess;
 }
 
 bool LoggingUnlockFile(FileHandle file) {