3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2024-12-27 15:32:10 +08:00
Code Issues Projects Releases Wiki Activity

Make NewReport objects own their associated database resources

Browse Source 
This change updates CrashReportDatbase::NewReport objects to own the
file handle associated with the new report, now accessible via a
FileWriter. NewReport's destructor closes its file handle and removes
its new report unless disarmed with FinishedWritingCrashReport,
eliminating the need for CallErrorWritingCrashReport.

Bug: crashpad:206
Change-Id: Iccb5bbc0ebadb07a237ff8eb938389afcfeae2a5
Reviewed-on: https://chromium-review.googlesource.com/916941
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
This commit is contained in:
Joshua Peraza 2018-02-13 17:28:11 -08:00 committed by Commit Bot
parent e5bbdaff87
commit c45ba7920e
11 changed files with 149 additions and 262 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
client/crash_report_database.cc
View File

@ -14,6 +14,8 @@
#include "client/crash_report_database.h"
#include "build/build_config.h"
namespace crashpad {
CrashReportDatabase::Report::Report()
@ -26,22 +28,31 @@ CrashReportDatabase::Report::Report()
upload_attempts(0),
upload_explicitly_requested(false) {}
CrashReportDatabase::CallErrorWritingCrashReport::CallErrorWritingCrashReport(
CrashReportDatabase* database,
NewReport* new_report)
: database_(database),
new_report_(new_report) {
}
CrashReportDatabase::NewReport::NewReport()
: writer_(std::make_unique<FileWriter>()), uuid_(), file_remover_() {}
CrashReportDatabase::CallErrorWritingCrashReport::
~CallErrorWritingCrashReport() {
if (new_report_) {
database_->ErrorWritingCrashReport(new_report_);
CrashReportDatabase::NewReport::~NewReport() = default;
bool CrashReportDatabase::NewReport::Initialize(
const base::FilePath& directory,
const base::FilePath::StringType& extension) {
if (!uuid_.InitializeWithNew()) {
return false;
}
}
void CrashReportDatabase::CallErrorWritingCrashReport::Disarm() {
new_report_ = nullptr;
#if defined(OS_WIN)
const std::wstring uuid_string = uuid_.ToString16();
#else
const std::string uuid_string = uuid_.ToString();
#endif
const base::FilePath path = directory.Append(uuid_string + extension);
if (!writer_->Open(
path, FileWriteMode::kCreateOrFail, FilePermissions::kOwnerOnly)) {
return false;
}
file_remover_.reset(path);
return true;
}
} // namespace crashpad

102
client/crash_report_database.h
View File

@ -24,6 +24,8 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "util/file/file_io.h"
#include "util/file/file_writer.h"
#include "util/file/scoped_remove_file.h"
#include "util/misc/metrics.h"
#include "util/misc/uuid.h"
@ -98,44 +100,32 @@ class CrashReportDatabase {
//! \brief A crash report that is in the process of being written.
//!
//! An instance of this struct should be created via PrepareNewCrashReport()
//! and destroyed with FinishedWritingCrashReport().
struct NewReport {
//! The file handle to which the report should be written.
FileHandle handle;
//! An instance of this struct should be created via PrepareNewCrashReport().
class NewReport {
public:
NewReport();
~NewReport();
//! An open FileWriter with which to write the report.
FileWriter* Writer() const { return writer_.get(); }
//! A unique identifier by which this report will always be known to the
//! database.
UUID uuid;
//! The path to the crash report being written.
base::FilePath path;
};
//! \brief A scoper to cleanly handle the interface requirement imposed by
//! PrepareNewCrashReport().
//!
//! Calls ErrorWritingCrashReport() upon destruction unless disarmed by
//! calling Disarm(). Armed upon construction.
class CallErrorWritingCrashReport {
public:
//! \brief Arms the object to call ErrorWritingCrashReport() on \a database
//! with an argument of \a new_report on destruction.
CallErrorWritingCrashReport(CrashReportDatabase* database,
NewReport* new_report);
//! \brief Calls ErrorWritingCrashReport() if the object is armed.
~CallErrorWritingCrashReport();
//! \brief Disarms the object so that CallErrorWritingCrashReport() will not
//! be called upon destruction.
void Disarm();
const UUID& ReportID() { return uuid_; }
private:
CrashReportDatabase* database_; // weak
NewReport* new_report_; // weak
friend class CrashReportDatabase;
friend class CrashReportDatabaseMac;
friend class CrashReportDatabaseWin;
DISALLOW_COPY_AND_ASSIGN(CallErrorWritingCrashReport);
bool Initialize(const base::FilePath& directory,
const base::FilePath::StringType& extension);
std::unique_ptr<FileWriter> writer_;
UUID uuid_;
ScopedRemoveFile file_remover_;
DISALLOW_COPY_AND_ASSIGN(NewReport);
};
//! \brief The result code for operations performed on a database.
@ -217,50 +207,32 @@ class CrashReportDatabase {
//! \brief Creates a record of a new crash report.
//!
//! Callers can then write the crash report using the file handle provided.
//! The caller does not own the new crash report record or its file handle,
//! both of which must be explicitly disposed of by calling
//! FinishedWritingCrashReport() or ErrorWritingCrashReport().
//! Callers should write the crash report using the FileWriter provided.
//! Callers should then call FinishedWritingCrashReport() to complete report
//! creation. If an error is encountered while writing the crash report, no
//! special action needs to be taken. If FinishedWritingCrashReport() is not
//! called, the report will be removed from the database when \a report is
//! destroyed.
//!
//! To arrange to call ErrorWritingCrashReport() during any early return, use
//! CallErrorWritingCrashReport.
//!
//! \param[out] report A NewReport object containing a file handle to which
//! the crash report data should be written. Only valid if this returns
//! #kNoError. The caller must not delete the NewReport object or close
//! the file handle within.
//! \param[out] report A NewReport object containing a FileWriter with which
//! to write the report data. Only valid if this returns #kNoError.
//!
//! \return The operation status code.
virtual OperationStatus PrepareNewCrashReport(NewReport** report) = 0;
virtual OperationStatus PrepareNewCrashReport(
std::unique_ptr<NewReport>* report) = 0;
//! \brief Informs the database that a crash report has been written.
//!
//! After calling this method, the database is permitted to move and rename
//! the file at NewReport::path.
//! \brief Informs the database that a crash report has been successfully
//! written.
//!
//! \param[in] report A NewReport obtained with PrepareNewCrashReport(). The
//! NewReport object and file handle within will be invalidated as part of
//! this call.
//! NewReport object will be invalidated as part of this call.
//! \param[out] uuid The UUID of this crash report.
//!
//! \return The operation status code.
virtual OperationStatus FinishedWritingCrashReport(NewReport* report,
virtual OperationStatus FinishedWritingCrashReport(
std::unique_ptr<NewReport> report,
UUID* uuid) = 0;
//! \brief Informs the database that an error occurred while attempting to
//! write a crash report, and that any resources associated with it should
//! be cleaned up.
//!
//! After calling this method, the database is permitted to remove the file at
//! NewReport::path.
//!
//! \param[in] report A NewReport obtained with PrepareNewCrashReport(). The
//! NewReport object and file handle within will be invalidated as part of
//! this call.
//!
//! \return The operation status code.
virtual OperationStatus ErrorWritingCrashReport(NewReport* report) = 0;
//! \brief Returns the crash report record for the unique identifier.
//!
//! \param[in] uuid The crash report record unique identifier.

94
client/crash_report_database_mac.mm
View File

@ -107,6 +107,8 @@ std::string XattrNameInternal(const base::StringPiece& name, bool new_name) {
name.data());
}
} // namespace
//! \brief A CrashReportDatabase that uses HFS+ extended attributes to store
//! report metadata.
//!
@ -130,10 +132,10 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
// CrashReportDatabase:
Settings* GetSettings() override;
OperationStatus PrepareNewCrashReport(NewReport** report) override;
OperationStatus FinishedWritingCrashReport(NewReport* report,
OperationStatus PrepareNewCrashReport(
std::unique_ptr<NewReport>* report) override;
OperationStatus FinishedWritingCrashReport(std::unique_ptr<NewReport> report,
UUID* uuid) override;
OperationStatus ErrorWritingCrashReport(NewReport* report) override;
OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override;
OperationStatus GetPendingReports(std::vector<Report>* reports) override;
OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
@ -301,103 +303,67 @@ Settings* CrashReportDatabaseMac::GetSettings() {
}
CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::PrepareNewCrashReport(NewReport** out_report) {
CrashReportDatabaseMac::PrepareNewCrashReport(
std::unique_ptr<NewReport>* out_report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
std::unique_ptr<NewReport> report(new NewReport());
uuid_t uuid_gen;
uuid_generate(uuid_gen);
report->uuid.InitializeFromBytes(uuid_gen);
report->path =
base_dir_.Append(kWriteDirectory)
.Append(report->uuid.ToString() + "." + kCrashReportFileExtension);
report->handle = HANDLE_EINTR(
open(report->path.value().c_str(),
O_WRONLY | O_EXLOCK | O_CREAT | O_EXCL | O_NOCTTY | O_CLOEXEC,
0600));
if (report->handle < 0) {
PLOG(ERROR) << "open " << report->path.value();
if (!report->Initialize(base_dir_.Append(kWriteDirectory),
std::string(".") + kCrashReportFileExtension)) {
return kFileSystemError;
}
// TODO(rsesek): Potentially use an fsetxattr() here instead.
if (!WriteXattr(
report->path, XattrName(kXattrUUID), report->uuid.ToString())) {
PLOG_IF(ERROR, IGNORE_EINTR(close(report->handle)) != 0) << "close";
if (!WriteXattr(report->file_remover_.get(),
XattrName(kXattrUUID),
report->ReportID().ToString())) {
return kDatabaseError;
}
*out_report = report.release();
out_report->reset(report.release());
return kNoError;
}
CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::FinishedWritingCrashReport(NewReport* report,
CrashReportDatabaseMac::FinishedWritingCrashReport(
std::unique_ptr<NewReport> report,
UUID* uuid) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
// Takes ownership of the |handle| and the O_EXLOCK.
base::ScopedFD lock(report->handle);
// Take ownership of the report.
std::unique_ptr<NewReport> scoped_report(report);
const base::FilePath& path = report->file_remover_.get();
// Get the report's UUID to return.
std::string uuid_string;
if (ReadXattr(report->path, XattrName(kXattrUUID),
&uuid_string) != XattrStatus::kOK ||
if (ReadXattr(path, XattrName(kXattrUUID), &uuid_string) !=
XattrStatus::kOK ||
!uuid->InitializeFromString(uuid_string)) {
LOG(ERROR) << "Failed to read UUID for crash report "
<< report->path.value();
LOG(ERROR) << "Failed to read UUID for crash report " << path.value();
return kDatabaseError;
}
if (*uuid != report->uuid) {
LOG(ERROR) << "UUID mismatch for crash report " << report->path.value();
if (*uuid != report->ReportID()) {
LOG(ERROR) << "UUID mismatch for crash report " << path.value();
return kDatabaseError;
}
// Record the creation time of this report.
if (!WriteXattrTimeT(report->path, XattrName(kXattrCreationTime),
time(nullptr))) {
if (!WriteXattrTimeT(path, XattrName(kXattrCreationTime), time(nullptr))) {
return kDatabaseError;
}
FileOffset size = report->Writer()->Seek(0, SEEK_END);
// Move the report to its new location for uploading.
base::FilePath new_path =
base_dir_.Append(kUploadPendingDirectory).Append(report->path.BaseName());
if (rename(report->path.value().c_str(), new_path.value().c_str()) != 0) {
PLOG(ERROR) << "rename " << report->path.value() << " to "
<< new_path.value();
base_dir_.Append(kUploadPendingDirectory).Append(path.BaseName());
if (rename(path.value().c_str(), new_path.value().c_str()) != 0) {
PLOG(ERROR) << "rename " << path.value() << " to " << new_path.value();
return kFileSystemError;
}
ignore_result(report->file_remover_.release());
Metrics::CrashReportPending(Metrics::PendingReportReason::kNewlyCreated);
Metrics::CrashReportSize(report->handle);
return kNoError;
}
CrashReportDatabase::OperationStatus
CrashReportDatabaseMac::ErrorWritingCrashReport(NewReport* report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
// Takes ownership of the |handle| and the O_EXLOCK.
base::ScopedFD lock(report->handle);
// Take ownership of the report.
std::unique_ptr<NewReport> scoped_report(report);
// Remove the file that the report would have been written to had no error
// occurred.
if (unlink(report->path.value().c_str()) != 0) {
PLOG(ERROR) << "unlink " << report->path.value();
return kFileSystemError;
}
Metrics::CrashReportSize(size);
return kNoError;
}
@ -774,8 +740,6 @@ std::unique_ptr<CrashReportDatabase> InitializeInternal(
return std::unique_ptr<CrashReportDatabase>(database_mac.release());
}
} // namespace
// static
std::unique_ptr<CrashReportDatabase> CrashReportDatabase::Initialize(
const base::FilePath& path) {

46
client/crash_report_database_test.cc
View File

@ -14,13 +14,13 @@
#include "client/crash_report_database.h"
#include "build/build_config.h"
#include "client/settings.h"
#include "gtest/gtest.h"
#include "test/errors.h"
#include "test/file.h"
#include "test/scoped_temp_dir.h"
#include "util/file/file_io.h"
#include "util/file/filesystem.h"
namespace crashpad {
namespace test {
@ -48,20 +48,19 @@ class CrashReportDatabaseTest : public testing::Test {
}
void CreateCrashReport(CrashReportDatabase::Report* report) {
CrashReportDatabase::NewReport* new_report = nullptr;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
ASSERT_EQ(db_->PrepareNewCrashReport(&new_report),
CrashReportDatabase::kNoError);
static constexpr char kTest[] = "test";
ASSERT_TRUE(LoggingWriteFile(new_report->handle, kTest, sizeof(kTest)));
ASSERT_TRUE(new_report->Writer()->Write(kTest, sizeof(kTest)));
UUID uuid;
EXPECT_EQ(db_->FinishedWritingCrashReport(new_report, &uuid),
EXPECT_EQ(db_->FinishedWritingCrashReport(std::move(new_report), &uuid),
CrashReportDatabase::kNoError);
EXPECT_EQ(db_->LookUpCrashReport(uuid, report),
CrashReportDatabase::kNoError);
ExpectPreparedCrashReport(*report);
ASSERT_TRUE(FileExists(report->file_path));
}
void UploadReport(const UUID& uuid, bool successful, const std::string& id) {
@ -176,13 +175,12 @@ TEST_F(CrashReportDatabaseTest, Initialize) {
}
TEST_F(CrashReportDatabaseTest, NewCrashReport) {
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
EXPECT_EQ(db()->PrepareNewCrashReport(&new_report),
CrashReportDatabase::kNoError);
UUID expect_uuid = new_report->uuid;
EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value();
UUID expect_uuid = new_report->ReportID();
UUID uuid;
EXPECT_EQ(db()->FinishedWritingCrashReport(new_report, &uuid),
EXPECT_EQ(db()->FinishedWritingCrashReport(std::move(new_report), &uuid),
CrashReportDatabase::kNoError);
EXPECT_EQ(uuid, expect_uuid);
@ -201,17 +199,6 @@ TEST_F(CrashReportDatabaseTest, NewCrashReport) {
EXPECT_TRUE(reports.empty());
}
TEST_F(CrashReportDatabaseTest, ErrorWritingCrashReport) {
CrashReportDatabase::NewReport* new_report = nullptr;
ASSERT_EQ(db()->PrepareNewCrashReport(&new_report),
CrashReportDatabase::kNoError);
base::FilePath new_report_path = new_report->path;
EXPECT_TRUE(FileExists(new_report_path)) << new_report_path.value();
EXPECT_EQ(db()->ErrorWritingCrashReport(new_report),
CrashReportDatabase::kNoError);
EXPECT_FALSE(FileExists(new_report_path)) << new_report_path.value();
}
TEST_F(CrashReportDatabaseTest, LookUpCrashReport) {
UUID uuid;
@ -495,12 +482,11 @@ TEST_F(CrashReportDatabaseTest, UploadAlreadyUploaded) {
}
TEST_F(CrashReportDatabaseTest, MoveDatabase) {
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
EXPECT_EQ(db()->PrepareNewCrashReport(&new_report),
CrashReportDatabase::kNoError);
EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value();
UUID uuid;
EXPECT_EQ(db()->FinishedWritingCrashReport(new_report, &uuid),
EXPECT_EQ(db()->FinishedWritingCrashReport(std::move(new_report), &uuid),
CrashReportDatabase::kNoError);
RelocateDatabase();
@ -509,28 +495,22 @@ TEST_F(CrashReportDatabaseTest, MoveDatabase) {
EXPECT_EQ(db()->LookUpCrashReport(uuid, &report),
CrashReportDatabase::kNoError);
ExpectPreparedCrashReport(report);
EXPECT_TRUE(FileExists(report.file_path)) << report.file_path.value();
}
TEST_F(CrashReportDatabaseTest, ReportRemoved) {
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
EXPECT_EQ(db()->PrepareNewCrashReport(&new_report),
CrashReportDatabase::kNoError);
EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value();
UUID uuid;
EXPECT_EQ(db()->FinishedWritingCrashReport(new_report, &uuid),
EXPECT_EQ(db()->FinishedWritingCrashReport(std::move(new_report), &uuid),
CrashReportDatabase::kNoError);
CrashReportDatabase::Report report;
EXPECT_EQ(db()->LookUpCrashReport(uuid, &report),
CrashReportDatabase::kNoError);
#if defined(OS_WIN)
EXPECT_EQ(_wunlink(report.file_path.value().c_str()), 0);
#else
EXPECT_EQ(unlink(report.file_path.value().c_str()), 0)
<< ErrnoMessage("unlink");
#endif
EXPECT_TRUE(LoggingRemoveFile(report.file_path));
EXPECT_EQ(db()->LookUpCrashReport(uuid, &report),
CrashReportDatabase::kReportNotFound);

65
client/crash_report_database_win.cc
View File

@ -571,6 +571,8 @@ bool CreateDirectoryIfNecessary(const base::FilePath& path) {
return EnsureDirectory(path);
}
} // namespace
// CrashReportDatabaseWin ------------------------------------------------------
class CrashReportDatabaseWin : public CrashReportDatabase {
@ -582,10 +584,10 @@ class CrashReportDatabaseWin : public CrashReportDatabase {
// CrashReportDatabase:
Settings* GetSettings() override;
OperationStatus PrepareNewCrashReport(NewReport** report) override;
OperationStatus FinishedWritingCrashReport(NewReport* report,
OperationStatus PrepareNewCrashReport(
std::unique_ptr<NewReport>* report) override;
OperationStatus FinishedWritingCrashReport(std::unique_ptr<NewReport> report,
UUID* uuid) override;
OperationStatus ErrorWritingCrashReport(NewReport* report) override;
OperationStatus LookUpCrashReport(const UUID& uuid, Report* report) override;
OperationStatus GetPendingReports(std::vector<Report>* reports) override;
OperationStatus GetCompletedReports(std::vector<Report>* reports) override;
@ -643,67 +645,38 @@ Settings* CrashReportDatabaseWin::GetSettings() {
}
OperationStatus CrashReportDatabaseWin::PrepareNewCrashReport(
NewReport** report) {
std::unique_ptr<NewReport>* report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
std::unique_ptr<NewReport> new_report(new NewReport());
if (!new_report->uuid.InitializeWithNew())
return kFileSystemError;
new_report->path = base_dir_.Append(kReportsDirectory)
.Append(new_report->uuid.ToString16() + L"." +
kCrashReportFileExtension);
new_report->handle = LoggingOpenFileForWrite(new_report->path,
FileWriteMode::kCreateOrFail,
FilePermissions::kOwnerOnly);
if (new_report->handle == INVALID_HANDLE_VALUE)
if (!new_report->Initialize(base_dir_.Append(kReportsDirectory),
std::wstring(L".") + kCrashReportFileExtension)) {
return kFileSystemError;
}
*report = new_report.release();
report->reset(new_report.release());
return kNoError;
}
OperationStatus CrashReportDatabaseWin::FinishedWritingCrashReport(
NewReport* report,
std::unique_ptr<NewReport> report,
UUID* uuid) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
// Take ownership of the report.
std::unique_ptr<NewReport> scoped_report(report);
// Take ownership of the file handle.
ScopedFileHandle handle(report->handle);
std::unique_ptr<Metadata> metadata(AcquireMetadata());
if (!metadata)
return kDatabaseError;
metadata->AddNewRecord(ReportDisk(scoped_report->uuid,
scoped_report->path,
metadata->AddNewRecord(ReportDisk(report->ReportID(),
report->file_remover_.get(),
time(nullptr),
ReportState::kPending));
*uuid = scoped_report->uuid;
ignore_result(report->file_remover_.release());
*uuid = report->ReportID();
Metrics::CrashReportPending(Metrics::PendingReportReason::kNewlyCreated);
Metrics::CrashReportSize(handle.get());
return kNoError;
}
OperationStatus CrashReportDatabaseWin::ErrorWritingCrashReport(
NewReport* report) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
// Take ownership of the report.
std::unique_ptr<NewReport> scoped_report(report);
// Close the outstanding handle.
LoggingCloseFile(report->handle);
// We failed to write, so remove the dump file. There's no entry in the
// metadata table yet.
if (!DeleteFile(scoped_report->path.value().c_str())) {
PLOG(ERROR) << "DeleteFile "
<< base::UTF16ToUTF8(scoped_report->path.value());
return kFileSystemError;
}
Metrics::CrashReportSize(report->Writer()->Seek(0, SEEK_END));
return kNoError;
}
@ -899,8 +872,6 @@ OperationStatus CrashReportDatabaseWin::RequestUpload(const UUID& uuid) {
return kNoError;
}
} // namespace
// static
std::unique_ptr<CrashReportDatabase> CrashReportDatabase::Initialize(
const base::FilePath& path) {

12
client/prune_crash_reports_test.cc
View File

@ -36,9 +36,8 @@ class MockDatabase : public CrashReportDatabase {
public:
// CrashReportDatabase:
MOCK_METHOD0(GetSettings, Settings*());
MOCK_METHOD1(PrepareNewCrashReport, OperationStatus(NewReport**));
MOCK_METHOD2(FinishedWritingCrashReport, OperationStatus(NewReport*, UUID*));
MOCK_METHOD1(ErrorWritingCrashReport, OperationStatus(NewReport*));
MOCK_METHOD1(PrepareNewCrashReport,
OperationStatus(std::unique_ptr<NewReport>*));
MOCK_METHOD2(LookUpCrashReport, OperationStatus(const UUID&, Report*));
MOCK_METHOD1(GetPendingReports, OperationStatus(std::vector<Report>*));
MOCK_METHOD1(GetCompletedReports, OperationStatus(std::vector<Report>*));
@ -50,6 +49,13 @@ class MockDatabase : public CrashReportDatabase {
OperationStatus(const UUID&, Metrics::CrashSkippedReason));
MOCK_METHOD1(DeleteReport, OperationStatus(const UUID&));
MOCK_METHOD1(RequestUpload, OperationStatus(const UUID&));
// gmock doesn't support mocking methods with non-copyable types such as
// unique_ptr.
OperationStatus FinishedWritingCrashReport(std::unique_ptr<NewReport> report,
UUID* uuid) override {
return kNoError;
}
};
time_t NDaysAgo(int num_days) {

17
handler/mac/crash_report_exception_handler.cc
View File

@ -14,6 +14,7 @@
#include "handler/mac/crash_report_exception_handler.h"
#include <utility>
#include <vector>
#include "base/logging.h"
@ -155,7 +156,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
process_snapshot.SetClientID(client_id);
process_snapshot.SetAnnotationsSimpleMap(*process_annotations_);
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
CrashReportDatabase::OperationStatus database_status =
database_->PrepareNewCrashReport(&new_report);
if (database_status != CrashReportDatabase::kNoError) {
@ -164,28 +165,22 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
return KERN_FAILURE;
}
process_snapshot.SetReportID(new_report->uuid);
CrashReportDatabase::CallErrorWritingCrashReport
call_error_writing_crash_report(database_, new_report);
WeakFileHandleFileWriter file_writer(new_report->handle);
process_snapshot.SetReportID(new_report->ReportID());
MinidumpFileWriter minidump;
minidump.InitializeFromSnapshot(&process_snapshot);
AddUserExtensionStreams(
user_stream_data_sources_, &process_snapshot, &minidump);
if (!minidump.WriteEverything(&file_writer)) {
if (!minidump.WriteEverything(new_report->Writer())) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kMinidumpWriteFailed);
return KERN_FAILURE;
}
call_error_writing_crash_report.Disarm();
UUID uuid;
database_status = database_->FinishedWritingCrashReport(new_report, &uuid);
database_status =
database_->FinishedWritingCrashReport(std::move(new_report), &uuid);
if (database_status != CrashReportDatabase::kNoError) {
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kFinishedWritingCrashReportFailed);

17
handler/win/crash_report_exception_handler.cc
View File

@ -15,6 +15,7 @@
#include "handler/win/crash_report_exception_handler.h"
#include <type_traits>
#include <utility>
#include "client/crash_report_database.h"
#include "client/settings.h"
@ -90,7 +91,7 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
process_snapshot.SetClientID(client_id);
process_snapshot.SetAnnotationsSimpleMap(*process_annotations_);
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
CrashReportDatabase::OperationStatus database_status =
database_->PrepareNewCrashReport(&new_report);
if (database_status != CrashReportDatabase::kNoError) {
@ -100,29 +101,23 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
return termination_code;
}
process_snapshot.SetReportID(new_report->uuid);
CrashReportDatabase::CallErrorWritingCrashReport
call_error_writing_crash_report(database_, new_report);
WeakFileHandleFileWriter file_writer(new_report->handle);
process_snapshot.SetReportID(new_report->ReportID());
MinidumpFileWriter minidump;
minidump.InitializeFromSnapshot(&process_snapshot);
AddUserExtensionStreams(
user_stream_data_sources_, &process_snapshot, &minidump);
if (!minidump.WriteEverything(&file_writer)) {
if (!minidump.WriteEverything(new_report->Writer())) {
LOG(ERROR) << "WriteEverything failed";
Metrics::ExceptionCaptureResult(
Metrics::CaptureResult::kMinidumpWriteFailed);
return termination_code;
}
call_error_writing_crash_report.Disarm();
UUID uuid;
database_status = database_->FinishedWritingCrashReport(new_report, &uuid);
database_status =
database_->FinishedWritingCrashReport(std::move(new_report), &uuid);
if (database_status != CrashReportDatabase::kNoError) {
LOG(ERROR) << "FinishedWritingCrashReport failed";
Metrics::ExceptionCaptureResult(

12
tools/crashpad_database_util.cc
View File

@ -584,16 +584,13 @@ int DatabaseUtilMain(int argc, char* argv[]) {
file_reader = std::move(file_path_reader);
}
CrashReportDatabase::NewReport* new_report;
std::unique_ptr<CrashReportDatabase::NewReport> new_report;
CrashReportDatabase::OperationStatus status =
database->PrepareNewCrashReport(&new_report);
if (status != CrashReportDatabase::kNoError) {
return EXIT_FAILURE;
}
CrashReportDatabase::CallErrorWritingCrashReport
call_error_writing_crash_report(database.get(), new_report);
char buf[4096];
FileOperationResult read_result;
do {
@ -601,16 +598,13 @@ int DatabaseUtilMain(int argc, char* argv[]) {
if (read_result < 0) {
return EXIT_FAILURE;
}
if (read_result > 0 &&
!LoggingWriteFile(new_report->handle, buf, read_result)) {
if (read_result > 0 && !new_report->Writer()->Write(buf, read_result)) {
return EXIT_FAILURE;
}
} while (read_result > 0);
call_error_writing_crash_report.Disarm();
UUID uuid;
status = database->FinishedWritingCrashReport(new_report, &uuid);
status = database->FinishedWritingCrashReport(std::move(new_report), &uuid);
if (status != CrashReportDatabase::kNoError) {
return EXIT_FAILURE;
}

3
util/misc/metrics.cc
View File

@ -61,8 +61,7 @@ void Metrics::CrashReportPending(PendingReportReason reason) {
}
// static
void Metrics::CrashReportSize(FileHandle file) {
const FileOffset size = LoggingFileSizeByHandle(file);
void Metrics::CrashReportSize(FileOffset size) {
UMA_HISTOGRAM_CUSTOM_COUNTS(
"Crashpad.CrashReportSize", size, 0, 20 * 1024 * 1024, 50);
}

2
util/misc/metrics.h
View File

@ -50,7 +50,7 @@ class Metrics {
//! \brief Reports the size of a crash report file in bytes. Should be called
//! when a new report is written to disk.
static void CrashReportSize(FileHandle file);
static void CrashReportSize(FileOffset size);
//! \brief Reports on a crash upload attempt, and if it succeeded.
static void CrashUploadAttempted(bool successful);