Use LockFile/UnlockFile for Settings to port to Windows

client/settings.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #include "client/settings.h"
 
 #include <limits>
 
-#include <fcntl.h>
-#include <unistd.h>
-#include <uuid/uuid.h>
-
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "util/numeric/in_range_cast.h"
 
 namespace crashpad {
 
 struct ALIGNAS(4) Settings::Data {
   static const uint32_t kSettingsMagic = 'CPds';
   static const uint32_t kSettingsVersion = 1;
@@ skipping 21 matching lines @@
     : file_path_(file_path),
       initialized_() {
 }
 
 Settings::~Settings() {
 }
 
 bool Settings::Initialize() {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
-  ScopedFileHandle handle(HANDLE_EINTR(

[scottmg, 2015/04/01 00:49:14]

I think the/a case that has changed here:

- X enters, attempts open, succeeds because the file didn't exist.
- Y enters, attempts open, fails because the file does exist.
- Y continues down to the "normal" case, opens for RW.
- Now they're racing for the lock.
- If X locks, all is well as it will initialize, release, and Y continues
- if Y locks, then ReadSettings will fail, causing RecoverSettings to
InitializeSettings (which is what we want), except after unlock, it'll happen
again on X which is a bit weird.

Which makes me think that simply deleting the "initialize" path would be
equivalent.

But, I'm not sure if I understand the subtlety of atomicity of O_EXLOCK or the
various conditions, so please correct me if I'm way off here.

[Robert Sesek, 2015/04/01 15:08:26]

I think RecoverSettings won't call InitializeSettings again, since it takes the
O_EXLOCK and then tries to read them again, to prevent the double-recovery case.
Yes, it could be. Non-existent file is technically just another error/recovery
condition, so I'd be OK with removing the entire Initialize step I think. The
only issue is that then when creating the database for the first time, there
will be some inaccurate (or misleading) log messages saying that we're trying to
"recover" the settings file.
Yes, this is the race condition I mentioned on the bug.

-      open(file_path().value().c_str(),
-           O_CREAT | O_EXCL | O_WRONLY | O_EXLOCK,
-           0644)));
+  ScopedLockedFileHandle handle(
+      LoggingOpenFileForWrite(file_path(),
+                              FileWriteMode::kCreateOrFail,
+                              FilePermissions::kWorldReadable),
+      FileLocking::kExclusive);
 
   // The file was created, so this is a new database that needs to be
   // initialized with a client ID.
   if (handle.is_valid()) {
     bool initialized = InitializeSettings(handle.get());
     if (initialized)
       INITIALIZATION_STATE_SET_VALID(initialized_);
     return initialized;
   }
 
@@ skipping 27 matching lines @@
     return false;
 
   *enabled = (settings.options & Data::Options::kUploadsEnabled) != 0;
   return true;
 }
 
 bool Settings::SetUploadsEnabled(bool enabled) {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
   Data settings;
-  ScopedFileHandle handle = OpenForWritingAndReadSettings(&settings);
+  ScopedLockedFileHandle handle = OpenForWritingAndReadSettings(&settings);
   if (!handle.is_valid())
     return false;
 
   if (enabled)
     settings.options |= Data::Options::kUploadsEnabled;
   else
     settings.options &= ~Data::Options::kUploadsEnabled;
 
   return WriteSettings(handle.get(), settings);
 }
 
 bool Settings::GetLastUploadAttemptTime(time_t* time) {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
   Data settings;
   if (!OpenAndReadSettings(&settings))
     return false;
 
   *time = InRangeCast<time_t>(settings.last_upload_attempt_time,
                               std::numeric_limits<time_t>::max());
   return true;
 }
 
 bool Settings::SetLastUploadAttemptTime(time_t time) {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
   Data settings;
-  ScopedFileHandle handle = OpenForWritingAndReadSettings(&settings);
+  ScopedLockedFileHandle handle = OpenForWritingAndReadSettings(&settings);
   if (!handle.is_valid())
     return false;
 
   settings.last_upload_attempt_time = InRangeCast<uint64_t>(time, 0);
 
   return WriteSettings(handle.get(), settings);
 }
 
-ScopedFileHandle Settings::OpenForReading() {
-  ScopedFileHandle handle(HANDLE_EINTR(
-      open(file_path().value().c_str(), O_RDONLY | O_SHLOCK)));
-  PLOG_IF(ERROR, !handle.is_valid()) << "open for reading";
+Settings::ScopedLockedFileHandle::ScopedLockedFileHandle() : handle_() {
+}
+
+Settings::ScopedLockedFileHandle::ScopedLockedFileHandle(FileHandle file,
+                                                         FileLocking locking)
+    : handle_(file) {
+  if (handle_.is_valid()) {
+    if (!LoggingLockFile(handle_.get(), locking))
+      handle_.reset();
+  }
+}
+
+Settings::ScopedLockedFileHandle::~ScopedLockedFileHandle() {
+  FreeIfNecessary();
+}
+
+Settings::ScopedLockedFileHandle Settings::OpenForReading() {
+  ScopedLockedFileHandle handle(LoggingOpenFileForRead(file_path()),
+                                FileLocking::kShared);
   return handle.Pass();
 }
 
-ScopedFileHandle Settings::OpenForReadingAndWriting() {
-  ScopedFileHandle handle(HANDLE_EINTR(
-      open(file_path().value().c_str(), O_RDWR | O_EXLOCK | O_CREAT, 0644)));
-  PLOG_IF(ERROR, !handle.is_valid()) << "open for writing";
+Settings::ScopedLockedFileHandle Settings::OpenForReadingAndWriting() {
+  ScopedLockedFileHandle handle(
+      LoggingOpenFileForReadAndWrite(file_path(),
+                                     FileWriteMode::kReuseOrCreate,
+                                     FilePermissions::kWorldReadable),
+      FileLocking::kExclusive);
   return handle.Pass();
 }
 
+void Settings::ScopedLockedFileHandle::reset() {
+  FreeIfNecessary();
+  handle_.reset();
+}
+
+void Settings::ScopedLockedFileHandle::FreeIfNecessary() {
+  if (is_valid())
+    LoggingUnlockFile(handle_.get());
+}
+
 bool Settings::OpenAndReadSettings(Data* out_data) {
-  ScopedFileHandle handle = OpenForReading();
+  ScopedLockedFileHandle handle = OpenForReading();
   if (!handle.is_valid())
     return false;
 
   if (ReadSettings(handle.get(), out_data))
     return true;
 
   // The settings file is corrupt, so reinitialize it.
   handle.reset();
 
   // The settings failed to be read, so re-initialize them.
   return RecoverSettings(kInvalidFileHandle, out_data);
 }
 
-ScopedFileHandle Settings::OpenForWritingAndReadSettings(Data* out_data) {
-  ScopedFileHandle handle = OpenForReadingAndWriting();
+Settings::ScopedLockedFileHandle Settings::OpenForWritingAndReadSettings(
+    Data* out_data) {
+  ScopedLockedFileHandle handle = OpenForReadingAndWriting();
   if (!handle.is_valid())
-    return ScopedFileHandle();
+    return ScopedLockedFileHandle();
 
   if (!ReadSettings(handle.get(), out_data)) {
     if (!RecoverSettings(handle.get(), out_data))
-      return ScopedFileHandle();
+      return ScopedLockedFileHandle();
   }
 
   return handle.Pass();
 }
 
 bool Settings::ReadSettings(FileHandle handle, Data* out_data) {
   if (LoggingSeekFile(handle, 0, SEEK_SET) != 0)
     return false;
 
   if (!LoggingReadFile(handle, out_data, sizeof(*out_data)))
     return false;
 
   if (out_data->magic != Data::kSettingsMagic) {
     LOG(ERROR) << "Settings magic is not " << Data::kSettingsMagic;
     return false;
   }
 
   if (out_data->version != Data::kSettingsVersion) {
     LOG(ERROR) << "Settings version is not " << Data::kSettingsVersion;
     return false;
   }
 
   return true;
 }
 
 bool Settings::WriteSettings(FileHandle handle, const Data& data) {
   if (LoggingSeekFile(handle, 0, SEEK_SET) != 0)
     return false;
 
-  if (HANDLE_EINTR(ftruncate(handle, 0)) != 0) {
-    PLOG(ERROR) << "ftruncate settings file";
+  if (!LoggingTruncateFile(handle))
     return false;
-  }
 
   return LoggingWriteFile(handle, &data, sizeof(Data));
 }
 
 bool Settings::RecoverSettings(FileHandle handle, Data* out_data) {
-  ScopedFileHandle scoped_handle;
+  ScopedLockedFileHandle scoped_handle;
   if (handle == kInvalidFileHandle) {
     scoped_handle = OpenForReadingAndWriting();
     handle = scoped_handle.get();
 
     // Test if the file has already been recovered now that the exclusive lock
     // is held.
     if (ReadSettings(handle, out_data))
       return true;
   }
 
-  LOG(INFO) << "Recovering settings file " << file_path().value();
+  LOG(INFO) << "Recovering settings file " << file_path().value().c_str();
 
   if (handle == kInvalidFileHandle) {
     LOG(ERROR) << "Invalid file handle";
     return false;
   }
 
   if (!InitializeSettings(handle))
     return false;
 
   return ReadSettings(handle, out_data);
 }
 
 bool Settings::InitializeSettings(FileHandle handle) {
-  uuid_t uuid;
-  uuid_generate(uuid);
-
   Data settings;
-  settings.client_id.InitializeFromBytes(uuid);
+  if (!UUID::GenerateFromSystem(&settings.client_id))
+    return false;
 
   return WriteSettings(handle, settings);
 }
 
 }  // namespace crashpad