Use LockFile/UnlockFile for Settings to port to Windows

Adds LoggingOpenFileForReadAndWrite, LoggingTruncateFile, and UUID::GenerateFromSystem
in support.

R=mark@chromium.org, rsesek@chromium.org
BUG=crashpad:13

Review URL: https://codereview.chromium.org/999953002
This commit is contained in:
Scott Graham 2015-04-20 14:21:48 -07:00
parent 1baff4ff92
commit 8272a4cefe
12 changed files with 244 additions and 109 deletions

View File

@ -54,10 +54,6 @@
'-lrpcrt4.lib',
],
},
'sources!': [
# Port to Win https://code.google.com/p/crashpad/issues/detail?id=13
'settings.cc',
],
}],
],
'direct_dependent_settings': {

View File

@ -39,14 +39,6 @@
'simple_string_dictionary_test.cc',
'simulate_crash_mac_test.cc',
],
'conditions': [
['OS=="win"', {
'sources!': [
# Port to Win https://code.google.com/p/crashpad/issues/detail?id=13
'settings_test.cc',
],
}],
],
},
],
}

View File

@ -14,7 +14,6 @@
#include "client/crash_report_database.h"
#include <rpc.h>
#include <string.h>
#include <time.h>
#include <windows.h>
@ -573,12 +572,9 @@ OperationStatus CrashReportDatabaseWin::PrepareNewCrashReport(
NewReport** report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
::UUID system_uuid;
if (UuidCreate(&system_uuid) != RPC_S_OK)
return kFileSystemError;
scoped_ptr<NewReport> new_report(new NewReport());
new_report->uuid.InitializeFromSystemUUID(&system_uuid);
if (!new_report->uuid.InitializeWithNew())
return kFileSystemError;
new_report->path = base_dir_.Append(kReportsDirectory)
.Append(new_report->uuid.ToString16() + L"." +
kCrashReportFileExtension);

View File

@ -16,10 +16,6 @@
#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"
@ -27,6 +23,23 @@
namespace crashpad {
namespace internal {
// static
FileHandle ScopedLockedFileHandleTraits::InvalidValue() {
return kInvalidFileHandle;
}
// static
void ScopedLockedFileHandleTraits::Free(FileHandle handle) {
if (handle != kInvalidFileHandle) {
LoggingUnlockFile(handle);
CheckedCloseFile(handle);
}
}
} // namespace internal
struct ALIGNAS(4) Settings::Data {
static const uint32_t kSettingsMagic = 'CPds';
static const uint32_t kSettingsVersion = 1;
@ -61,23 +74,6 @@ Settings::~Settings() {
bool Settings::Initialize() {
INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
ScopedFileHandle handle(HANDLE_EINTR(
open(file_path().value().c_str(),
O_CREAT | O_EXCL | O_WRONLY | O_EXLOCK,
0644)));
// 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;
}
// The file wasn't created, try opening it for a write operation. If the file
// needs to be recovered, writing is necessary. This also ensures that the
// process has permission to write the file.
Data settings;
if (!OpenForWritingAndReadSettings(&settings).is_valid())
return false;
@ -112,7 +108,7 @@ 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;
@ -140,7 +136,7 @@ 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;
@ -149,22 +145,33 @@ bool Settings::SetLastUploadAttemptTime(time_t time) {
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";
return handle.Pass();
// static
Settings::ScopedLockedFileHandle Settings::MakeScopedLockedFileHandle(
FileHandle file,
FileLocking locking) {
ScopedFileHandle scoped(file);
if (scoped.is_valid()) {
if (!LoggingLockFile(scoped.get(), locking))
scoped.reset();
}
return ScopedLockedFileHandle(scoped.release());
}
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";
return handle.Pass();
Settings::ScopedLockedFileHandle Settings::OpenForReading() {
return MakeScopedLockedFileHandle(LoggingOpenFileForRead(file_path()),
FileLocking::kShared);
}
Settings::ScopedLockedFileHandle Settings::OpenForReadingAndWriting() {
return MakeScopedLockedFileHandle(
LoggingOpenFileForReadAndWrite(file_path(),
FileWriteMode::kReuseOrCreate,
FilePermissions::kWorldReadable),
FileLocking::kExclusive);
}
bool Settings::OpenAndReadSettings(Data* out_data) {
ScopedFileHandle handle = OpenForReading();
ScopedLockedFileHandle handle = OpenForReading();
if (!handle.is_valid())
return false;
@ -178,14 +185,15 @@ bool Settings::OpenAndReadSettings(Data* out_data) {
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();
@ -215,16 +223,14 @@ 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();
@ -235,8 +241,6 @@ bool Settings::RecoverSettings(FileHandle handle, Data* out_data) {
return true;
}
LOG(INFO) << "Recovering settings file " << file_path().value();
if (handle == kInvalidFileHandle) {
LOG(ERROR) << "Invalid file handle";
return false;
@ -249,11 +253,9 @@ bool Settings::RecoverSettings(FileHandle handle, Data* out_data) {
}
bool Settings::InitializeSettings(FileHandle handle) {
uuid_t uuid;
uuid_generate(uuid);
Data settings;
settings.client_id.InitializeFromBytes(uuid);
if (!settings.client_id.InitializeWithNew())
return false;
return WriteSettings(handle, settings);
}

View File

@ -21,12 +21,22 @@
#include "base/basictypes.h"
#include "base/files/file_path.h"
#include "base/scoped_generic.h"
#include "util/file/file_io.h"
#include "util/misc/initialization_state_dcheck.h"
#include "util/misc/uuid.h"
namespace crashpad {
namespace internal {
struct ScopedLockedFileHandleTraits {
static FileHandle InvalidValue();
static void Free(FileHandle handle);
};
} // namespace internal
//! \brief An interface for accessing and modifying the settings of a
//! CrashReportDatabase.
//!
@ -95,13 +105,21 @@ class Settings {
private:
struct Data;
// This must be constructed with MakeScopedLockedFileHandle(). It both unlocks
// and closes the file on destruction.
using ScopedLockedFileHandle =
base::ScopedGeneric<FileHandle,
internal::ScopedLockedFileHandleTraits>;
static ScopedLockedFileHandle MakeScopedLockedFileHandle(FileHandle file,
FileLocking locking);
// Opens the settings file for reading. On error, logs a message and returns
// the invalid handle.
ScopedFileHandle OpenForReading();
ScopedLockedFileHandle OpenForReading();
// Opens the settings file for reading and writing. On error, logs a message
// and returns the invalid handle.
ScopedFileHandle OpenForReadingAndWriting();
ScopedLockedFileHandle OpenForReadingAndWriting();
// Opens the settings file and reads the data. If that fails, an error will
// be logged and the settings will be recovered and re-initialized. If that
@ -111,7 +129,7 @@ class Settings {
// Opens the settings file for writing and reads the data. If reading fails,
// recovery is attempted. Returns the opened file handle on success, or the
// invalid file handle on failure, with an error logged.
ScopedFileHandle OpenForWritingAndReadSettings(Data* out_data);
ScopedLockedFileHandle OpenForWritingAndReadSettings(Data* out_data);
// Reads the settings from |handle|. Logs an error and returns false on
// failure. This does not perform recovery.

View File

@ -15,6 +15,7 @@
#include "client/settings.h"
#include "gtest/gtest.h"
#include "test/errors.h"
#include "test/scoped_temp_dir.h"
#include "util/file/file_io.h"
@ -27,7 +28,7 @@ class SettingsTest : public testing::Test {
SettingsTest() : settings_(settings_path()) {}
base::FilePath settings_path() {
return temp_dir_.path().Append("settings");
return temp_dir_.path().Append(FILE_PATH_LITERAL("settings"));
}
Settings* settings() { return &settings_; }
@ -151,7 +152,13 @@ TEST_F(SettingsTest, UnlinkFile) {
EXPECT_TRUE(settings()->SetUploadsEnabled(true));
EXPECT_TRUE(settings()->SetLastUploadAttemptTime(time(nullptr)));
EXPECT_EQ(0, unlink(settings_path().value().c_str()));
#if defined(OS_WIN)
EXPECT_EQ(0, _wunlink(settings_path().value().c_str()))
<< ErrnoMessage("_wunlink");
#else
EXPECT_EQ(0, unlink(settings_path().value().c_str()))
<< ErrnoMessage("unlink");
#endif
Settings local_settings(settings_path());
EXPECT_TRUE(local_settings.Initialize());

View File

@ -188,9 +188,9 @@ FileHandle LoggingOpenFileForRead(const base::FilePath& path);
//! \brief Wraps `open()` or `CreateFile()`, creating a file for output. Logs
//! an error if the operation fails.
//!
//! \a write_mode determines the style (truncate, reuse, etc.) that is used to
//! open the file. On POSIX, \a permissions determines the value that is passed
//! as `mode` to `open()`. On Windows, the file is always opened in binary mode
//! \a mode determines the style (truncate, reuse, etc.) that is used to open
//! the file. On POSIX, \a permissions determines the value that is passed as
//! `mode` to `open()`. On Windows, the file is always opened in binary mode
//! (that is, no CRLF translation). On Windows, the file is opened for sharing,
//! see LoggingLockFile() and LoggingUnlockFile() to control concurrent access.
//!
@ -200,9 +200,27 @@ FileHandle LoggingOpenFileForRead(const base::FilePath& path);
//! \sa FilePermissions
//! \sa ScopedFileHandle
FileHandle LoggingOpenFileForWrite(const base::FilePath& path,
FileWriteMode write_mode,
FileWriteMode mode,
FilePermissions permissions);
//! \brief Wraps `open()` or `CreateFile()`, creating a file for both input and
//! output. Logs an error if the operation fails.
//!
//! \a mode determines the style (truncate, reuse, etc.) that is used to open
//! the file. On POSIX, \a permissions determines the value that is passed as
//! `mode` to `open()`. On Windows, the file is always opened in binary mode
//! (that is, no CRLF translation). On Windows, the file is opened for sharing,
//! see LoggingLockFile() and LoggingUnlockFile() to control concurrent access.
//!
//! \return The newly opened FileHandle, or an invalid FileHandle on failure.
//!
//! \sa FileWriteMode
//! \sa FilePermissions
//! \sa ScopedFileHandle
FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions);
//! \brief Locks the given \a file using `flock()` on POSIX or `LockFileEx()` on
//! Windows.
//!
@ -243,6 +261,11 @@ bool LoggingUnlockFile(FileHandle file);
//! `-1` on failure.
FileOffset LoggingSeekFile(FileHandle file, FileOffset offset, int whence);
//! \brief Truncates the given \a file to zero bytes in length.
//!
//! \return `true` on success, or `false`, and a message will be logged.
bool LoggingTruncateFile(FileHandle file);
//! \brief Wraps `close()` or `CloseHandle()`, logging an error if the operation
//! fails.
//!

View File

@ -69,6 +69,29 @@ ssize_t ReadOrWrite(int fd,
namespace crashpad {
namespace {
FileHandle LoggingOpenFileForOutput(int rdwr_or_wronly,
const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
int flags = rdwr_or_wronly | O_CREAT;
// kReuseOrCreate does not need any additional flags.
if (mode == FileWriteMode::kTruncateOrCreate)
flags |= O_TRUNC;
else if (mode == FileWriteMode::kCreateOrFail)
flags |= O_EXCL;
int fd = HANDLE_EINTR(
open(path.value().c_str(),
flags,
permissions == FilePermissions::kWorldReadable ? 0644 : 0600));
PLOG_IF(ERROR, fd < 0) << "open " << path.value();
return fd;
}
} // namespace
ssize_t ReadFile(FileHandle file, void* buffer, size_t size) {
return ReadOrWrite<ReadTraits>(file, buffer, size);
}
@ -86,19 +109,13 @@ FileHandle LoggingOpenFileForRead(const base::FilePath& path) {
FileHandle LoggingOpenFileForWrite(const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
int flags = O_WRONLY | O_CREAT;
// kReuseOrCreate does not need any additional flags.
if (mode == FileWriteMode::kTruncateOrCreate)
flags |= O_TRUNC;
else if (mode == FileWriteMode::kCreateOrFail)
flags |= O_EXCL;
return LoggingOpenFileForOutput(O_WRONLY, path, mode, permissions);
}
int fd = HANDLE_EINTR(
open(path.value().c_str(),
flags,
permissions == FilePermissions::kWorldReadable ? 0644 : 0600));
PLOG_IF(ERROR, fd < 0) << "open " << path.value();
return fd;
FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
return LoggingOpenFileForOutput(O_RDWR, path, mode, permissions);
}
bool LoggingLockFile(FileHandle file, FileLocking locking) {
@ -120,6 +137,14 @@ FileOffset LoggingSeekFile(FileHandle file, FileOffset offset, int whence) {
return rv;
}
bool LoggingTruncateFile(FileHandle file) {
if (HANDLE_EINTR(ftruncate(file, 0)) != 0) {
PLOG(ERROR) << "ftruncate";
return false;
}
return true;
}
bool LoggingCloseFile(FileHandle file) {
int rv = IGNORE_EINTR(close(file));
PLOG_IF(ERROR, rv != 0) << "close";

View File

@ -34,6 +34,38 @@ bool IsSocketHandle(HANDLE file) {
namespace crashpad {
namespace {
FileHandle LoggingOpenFileForOutput(DWORD access,
const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
DWORD disposition = 0;
switch (mode) {
case FileWriteMode::kReuseOrCreate:
disposition = OPEN_ALWAYS;
break;
case FileWriteMode::kTruncateOrCreate:
disposition = CREATE_ALWAYS;
break;
case FileWriteMode::kCreateOrFail:
disposition = CREATE_NEW;
break;
}
HANDLE file = CreateFile(path.value().c_str(),
access,
FILE_SHARE_READ | FILE_SHARE_WRITE,
nullptr,
disposition,
FILE_ATTRIBUTE_NORMAL,
nullptr);
PLOG_IF(ERROR, file == INVALID_HANDLE_VALUE) << "CreateFile "
<< path.value().c_str();
return file;
}
} // namespace
// TODO(scottmg): Handle > DWORD sized writes if necessary.
ssize_t ReadFile(FileHandle file, void* buffer, size_t size) {
@ -95,28 +127,14 @@ FileHandle LoggingOpenFileForRead(const base::FilePath& path) {
FileHandle LoggingOpenFileForWrite(const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
DWORD disposition = 0;
switch (mode) {
case FileWriteMode::kReuseOrCreate:
disposition = OPEN_ALWAYS;
break;
case FileWriteMode::kTruncateOrCreate:
disposition = CREATE_ALWAYS;
break;
case FileWriteMode::kCreateOrFail:
disposition = CREATE_NEW;
break;
}
HANDLE file = CreateFile(path.value().c_str(),
GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE,
nullptr,
disposition,
FILE_ATTRIBUTE_NORMAL,
nullptr);
PLOG_IF(ERROR, file == INVALID_HANDLE_VALUE) << "CreateFile "
<< path.value().c_str();
return file;
return LoggingOpenFileForOutput(GENERIC_WRITE, path, mode, permissions);
}
FileHandle LoggingOpenFileForReadAndWrite(const base::FilePath& path,
FileWriteMode mode,
FilePermissions permissions) {
return LoggingOpenFileForOutput(
GENERIC_READ | GENERIC_WRITE, path, mode, permissions);
}
bool LoggingLockFile(FileHandle file, FileLocking locking) {
@ -174,6 +192,16 @@ FileOffset LoggingSeekFile(FileHandle file, FileOffset offset, int whence) {
return new_offset.QuadPart;
}
bool LoggingTruncateFile(FileHandle file) {
if (LoggingSeekFile(file, 0, SEEK_SET) != 0)
return false;
if (!SetEndOfFile(file)) {
PLOG(ERROR) << "SetEndOfFile";
return false;
}
return true;
}
bool LoggingCloseFile(FileHandle file) {
BOOL rv = CloseHandle(file);
PLOG_IF(ERROR, !rv) << "CloseHandle";

View File

@ -23,11 +23,16 @@
#include <string.h>
#include "base/basictypes.h"
#include "base/logging.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/sys_byteorder.h"
#include "util/stdlib/cxx.h"
#if defined(OS_MACOSX)
#include <uuid/uuid.h>
#endif // OS_MACOSX
#if CXX_LIBRARY_VERSION >= 2011
#include <type_traits>
#endif
@ -44,6 +49,10 @@ static_assert(std::is_standard_layout<UUID>::value,
UUID::UUID() : data_1(0), data_2(0), data_3(0), data_4(), data_5() {
}
UUID::UUID(InitializeWithNewTag) {
CHECK(InitializeWithNew());
}
UUID::UUID(const uint8_t* bytes) {
InitializeFromBytes(bytes);
}
@ -88,6 +97,25 @@ bool UUID::InitializeFromString(const base::StringPiece& string) {
return true;
}
bool UUID::InitializeWithNew() {
#if defined(OS_MACOSX)
uuid_t uuid;
uuid_generate(uuid);
InitializeFromBytes(uuid);
return true;
#elif defined(OS_WIN)
::UUID system_uuid;
if (UuidCreate(&system_uuid) != RPC_S_OK) {
LOG(ERROR) << "UuidCreate";
return false;
}
InitializeFromSystemUUID(&system_uuid);
return true;
#else
#error Port.
#endif // OS_MACOSX
}
#if defined(OS_WIN)
void UUID::InitializeFromSystemUUID(const ::UUID* system_uuid) {
static_assert(sizeof(::UUID) == sizeof(UUID),

View File

@ -41,6 +41,16 @@ struct UUID {
//! \brief Initializes the %UUID to zero.
UUID();
//! \brief Tag to pass to constructor to indicate it should initialize with
//! generated data.
struct InitializeWithNewTag {};
//! \brief Initializes the %UUID using a standard system facility to generate
//! the value.
//!
//! CHECKs on failure with a message logged.
explicit UUID(InitializeWithNewTag);
//! \copydoc InitializeFromBytes()
explicit UUID(const uint8_t* bytes);
@ -67,6 +77,13 @@ struct UUID {
//! parsed, with the object state untouched.
bool InitializeFromString(const base::StringPiece& string);
//! \brief Initializes the %UUID using a standard system facility to generate
//! the value.
//!
//! \return `true` if the %UUID was initialized correctly, `false` otherwise
//! with a message logged.
bool InitializeWithNew();
#if defined(OS_WIN) || DOXYGEN
//! \brief Initializes the %UUID from a system `UUID` or `GUID` structure.
//!

View File

@ -153,6 +153,9 @@ TEST(UUID, UUID) {
EXPECT_EQ(0x45u, uuid.data_5[4]);
EXPECT_EQ(0x45u, uuid.data_5[5]);
EXPECT_EQ("45454545-4545-4545-4545-454545454545", uuid.ToString());
UUID initialized_generated(UUID::InitializeWithNewTag{});
EXPECT_NE(initialized_generated, uuid_zero);
}
TEST(UUID, FromString) {