Commit 71834164 authored by Sigurdur Asgeirsson's avatar Sigurdur Asgeirsson Committed by Commit Bot

Initialize the atomic counters and pass them to the crashing function.

Also test the counters for sanity, turns out nothing's so simple
that it can't be messed up. While it's possible to retrieve the
value of the counters from a minidump with a debugger, it's quite
awkward in practice.

Bug: 1011441
Change-Id: I049eec2e668f61e8c2a7faa200fbde83441944d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019405
Commit-Queue: Ken Rockot <rockot@google.com>
Auto-Submit: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#735020}
parent e1a14d53
...@@ -40,7 +40,10 @@ NOINLINE void MaybeDumpWithoutCrashing( ...@@ -40,7 +40,10 @@ NOINLINE void MaybeDumpWithoutCrashing(
size_t total_quota_used, size_t total_quota_used,
base::Optional<size_t> message_pipe_quota_used, base::Optional<size_t> message_pipe_quota_used,
int64_t seconds_since_construction, int64_t seconds_since_construction,
double average_write_rate) { double average_write_rate,
uint64_t messages_enqueued,
uint64_t messages_dequeued,
uint64_t messages_written) {
static bool have_crashed = false; static bool have_crashed = false;
if (have_crashed) if (have_crashed)
return; return;
...@@ -66,6 +69,14 @@ NOINLINE void MaybeDumpWithoutCrashing( ...@@ -66,6 +69,14 @@ NOINLINE void MaybeDumpWithoutCrashing(
base::debug::Alias(&seconds_since_construction); base::debug::Alias(&seconds_since_construction);
base::debug::Alias(&average_write_rate_per_second); base::debug::Alias(&average_write_rate_per_second);
// Note that these values are acquired non-atomically with respect to the
// variables above, and so may have increased since the quota overflow
// occurred. They will still give a good indication of the traffic and the
// traffic mix on this MessageQuotaChecker.
base::debug::Alias(&messages_enqueued);
base::debug::Alias(&messages_dequeued);
base::debug::Alias(&messages_written);
// This is happening because the user of the interface implicated on the crash // This is happening because the user of the interface implicated on the crash
// stack has queued up an unreasonable number of messages, namely // stack has queued up an unreasonable number of messages, namely
// |total_quota_used|. // |total_quota_used|.
...@@ -245,9 +256,10 @@ void MessageQuotaChecker::QuotaCheckImpl(size_t num_enqueued) { ...@@ -245,9 +256,10 @@ void MessageQuotaChecker::QuotaCheckImpl(size_t num_enqueued) {
total_quota_used >= config_->crash_threshold) { total_quota_used >= config_->crash_threshold) {
DCHECK(!now.is_null()); DCHECK(!now.is_null());
int64_t seconds_since_construction = (now - creation_time_).InSeconds(); int64_t seconds_since_construction = (now - creation_time_).InSeconds();
config_->maybe_crash_function(total_quota_used, message_pipe_quota_used, config_->maybe_crash_function(
seconds_since_construction, total_quota_used, message_pipe_quota_used, seconds_since_construction,
average_write_rate); average_write_rate, messages_enqueued_.load(),
messages_dequeued_.load(), messages_written_.load());
} }
} }
......
...@@ -118,9 +118,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MessageQuotaChecker ...@@ -118,9 +118,9 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MessageQuotaChecker
// Cumulative counts for the number of messages enqueued with // Cumulative counts for the number of messages enqueued with
// |BeforeMessagesEnqueued()| and dequeued with |BeforeMessagesDequeued()|. // |BeforeMessagesEnqueued()| and dequeued with |BeforeMessagesDequeued()|.
std::atomic<uint64_t> messages_enqueued_; std::atomic<uint64_t> messages_enqueued_{0};
std::atomic<uint64_t> messages_dequeued_; std::atomic<uint64_t> messages_dequeued_{0};
std::atomic<uint64_t> messages_written_; std::atomic<uint64_t> messages_written_{0};
// A decaying average of the rate of call to BeforeWrite per second. // A decaying average of the rate of call to BeforeWrite per second.
DecayingRateAverage write_rate_average_; DecayingRateAverage write_rate_average_;
...@@ -142,7 +142,10 @@ struct MessageQuotaChecker::Configuration { ...@@ -142,7 +142,10 @@ struct MessageQuotaChecker::Configuration {
void (*maybe_crash_function)(size_t quota_used, void (*maybe_crash_function)(size_t quota_used,
base::Optional<size_t> message_pipe_quota_used, base::Optional<size_t> message_pipe_quota_used,
int64_t seconds_since_construction, int64_t seconds_since_construction,
double average_write_rate); double average_write_rate,
uint64_t messages_enqueued,
uint64_t messages_dequeued,
uint64_t messages_written);
}; };
} // namespace internal } // namespace internal
......
...@@ -43,12 +43,18 @@ class MessageQuotaCheckerTest : public testing::Test { ...@@ -43,12 +43,18 @@ class MessageQuotaCheckerTest : public testing::Test {
static void RecordDumpAttempt(size_t total_quota_used, static void RecordDumpAttempt(size_t total_quota_used,
base::Optional<size_t> message_pipe_quota_used, base::Optional<size_t> message_pipe_quota_used,
int64_t seconds_since_construction, int64_t seconds_since_construction,
double average_write_rate) { double average_write_rate,
uint64_t messages_enqueued,
uint64_t messages_dequeued,
uint64_t messages_written) {
++instance_->num_dumps_; ++instance_->num_dumps_;
instance_->last_dump_total_quota_used_ = total_quota_used; instance_->last_dump_total_quota_used_ = total_quota_used;
instance_->last_dump_message_pipe_quota_used_ = message_pipe_quota_used; instance_->last_dump_message_pipe_quota_used_ = message_pipe_quota_used;
instance_->last_seconds_since_construction_ = seconds_since_construction; instance_->last_seconds_since_construction_ = seconds_since_construction;
instance_->last_average_write_rate_ = average_write_rate; instance_->last_average_write_rate_ = average_write_rate;
instance_->last_messages_enqueued_ = messages_enqueued;
instance_->last_messages_dequeued_ = messages_dequeued;
instance_->last_messages_written_ = messages_written;
} }
// Mock time to allow testing // Mock time to allow testing
...@@ -60,6 +66,9 @@ class MessageQuotaCheckerTest : public testing::Test { ...@@ -60,6 +66,9 @@ class MessageQuotaCheckerTest : public testing::Test {
base::Optional<size_t> last_dump_message_pipe_quota_used_; base::Optional<size_t> last_dump_message_pipe_quota_used_;
int64_t last_seconds_since_construction_ = 0; int64_t last_seconds_since_construction_ = 0;
double last_average_write_rate_ = 0.0; double last_average_write_rate_ = 0.0;
uint64_t last_messages_enqueued_ = 0u;
uint64_t last_messages_dequeued_ = 0u;
uint64_t last_messages_written_ = 0u;
static const Configuration enabled_config_; static const Configuration enabled_config_;
...@@ -222,13 +231,17 @@ TEST_F(MessageQuotaCheckerTest, DumpsCoreOnOverrun) { ...@@ -222,13 +231,17 @@ TEST_F(MessageQuotaCheckerTest, DumpsCoreOnOverrun) {
Advance(kOneSamplingInterval); Advance(kOneSamplingInterval);
checker->SetMessagePipe(mojo::MessagePipeHandle()); checker->SetMessagePipe(mojo::MessagePipeHandle());
checker->BeforeMessagesEnqueued(250); checker->AfterMessagesDequeued(50);
checker->BeforeMessagesEnqueued(300);
EXPECT_EQ(3u, num_dumps_); EXPECT_EQ(3u, num_dumps_);
EXPECT_EQ(350u, last_dump_total_quota_used_); EXPECT_EQ(350u, last_dump_total_quota_used_);
EXPECT_FALSE(last_dump_message_pipe_quota_used_.has_value()); EXPECT_FALSE(last_dump_message_pipe_quota_used_.has_value());
EXPECT_EQ((12 * kOneSamplingInterval).InSeconds(), EXPECT_EQ((12 * kOneSamplingInterval).InSeconds(),
last_seconds_since_construction_); last_seconds_since_construction_);
EXPECT_EQ(12.75, last_average_write_rate_); EXPECT_EQ(12.75, last_average_write_rate_);
EXPECT_EQ(400u, last_messages_enqueued_);
EXPECT_EQ(50u, last_messages_dequeued_);
EXPECT_EQ(101u, last_messages_written_);
} }
TEST_F(MessageQuotaCheckerTest, DecayingRateAverage) { TEST_F(MessageQuotaCheckerTest, DecayingRateAverage) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment