cryptohome: correct ParallelTasks counter
We forget to increase the ParallelTasks counter in some location.
It would let the counter become negative number, and underflow the value
when report it to UMA.
And we also forget to initialize the parallel_task_count_.
BUG=b:169623257
TEST=Manually enroll, login on hatch, and check the value is correct in
chrome://histograms
Change-Id: I9c00bb02b38d7eae523eba81b0ebe0d0a651cbff
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2438169
Tested-by: joe Chou <yich@google.com>
Commit-Queue: joe Chou <yich@google.com>
Reviewed-by: Leo Lai <cylai@google.com>
diff --git a/cryptohome/service.cc b/cryptohome/service.cc
index 06c567b..83368bc 100644
--- a/cryptohome/service.cc
+++ b/cryptohome/service.cc
@@ -82,6 +82,11 @@
using brillo::SecureBlob;
using brillo::cryptohome::home::SanitizeUserNameWithSalt;
+namespace {
+constexpr const char* kIgnoreParallelTaskNames[] = {"LowDiskCallback",
+ "UploadAlertsDataCallback"};
+}
+
// Forcibly namespace the dbus-bindings generated server bindings instead of
// modifying the files afterward.
namespace cryptohome {
@@ -114,7 +119,7 @@
}
void AddTaskObserverToThread(base::Thread* thread,
- base::TaskObserver* task_observer) {
+ MountThreadObserver* task_observer) {
// Since MessageLoopCurrent::AddTaskObserver need to be executed in the same
// thread of that message loop. So we need to wrap it and post as a task.
@@ -125,10 +130,11 @@
return;
}
+ task_observer->PostTask();
task_runner->PostTask(
FROM_HERE,
base::BindOnce(
- [](base::TaskObserver* task_observer) {
+ [](MountThreadObserver* task_observer) {
base::MessageLoopCurrent::Get().AddTaskObserver(task_observer);
},
base::Unretained(task_observer)));
@@ -240,6 +246,11 @@
void MountThreadObserver::DidProcessTask(
const base::PendingTask& pending_task) {
+ for (const char* name : kIgnoreParallelTaskNames) {
+ if (pending_task.posted_from.function_name() == name) {
+ return;
+ }
+ }
parallel_task_count_ -= 1;
}
@@ -380,11 +391,11 @@
void Service::PostTask(const base::Location& from_here,
base::OnceClosure task) {
+ mount_thread_observer_.PostTask();
int task_count = mount_thread_observer_.GetParallelTaskCount();
if (task_count > 1) {
ReportParallelTasks(task_count);
}
- mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(from_here, std::move(task));
}
@@ -889,6 +900,7 @@
ReportTimerStart(kPkcs11InitTimer);
mount->set_pkcs11_state(cryptohome::Mount::kIsBeingInitialized);
+ mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&Service::DoInitializePkcs11, base::Unretained(this),
@@ -930,6 +942,8 @@
ReportAlertsData(alerts);
}
+ // We don't care about the parallel delay tasks number. Don't increase the
+ // parallel tasks count here.
mount_thread_.task_runner()->PostDelayedTask(
FROM_HERE,
base::Bind(&Service::UploadAlertsDataCallback, base::Unretained(this)),
@@ -1055,6 +1069,7 @@
{
base::AutoLock _lock(mounts_lock_);
for (const auto& mount_pair : mounts_) {
+ mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(
FROM_HERE,
base::Bind(&Service::DoResetTPMContext, base::Unretained(this),
@@ -3010,6 +3025,7 @@
if (!request_pb->ParseFromArray(request->data, request->len))
request_pb.reset(nullptr);
+ mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(
FROM_HERE, base::Bind(&Service::DoUnmountEx, base::Unretained(this),
base::Passed(std::move(request_pb)),
@@ -3449,6 +3465,7 @@
gboolean Service::TpmAttestationGetEnrollmentPreparationsEx(
const GArray* request, DBusGMethodInvocation* context) {
+ mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(
FROM_HERE,
base::Bind(&Service::DoTpmAttestationGetEnrollmentPreparationsEx,
@@ -3822,6 +3839,8 @@
// Schedule our next call. If the thread is terminating, we would
// not be called. We use base::Unretained here because the Service object is
// never destroyed.
+ // We don't care about the parallel delay tasks number. Don't increase the
+ // parallel tasks count here.
mount_thread_.task_runner()->PostDelayedTask(
FROM_HERE, base::Bind(&Service::LowDiskCallback, base::Unretained(this)),
base::TimeDelta::FromMilliseconds(low_disk_notification_period_ms_));
@@ -4198,6 +4217,7 @@
const std::string obfuscated_username = SanitizeUserNameWithSalt(
GetAccountId(request_pb.account_id()), system_salt_);
+ mount_thread_observer_.PostTask();
mount_thread_.task_runner()->PostTask(
FROM_HERE, base::Bind(&Service::DoLockToSingleUserMountUntilReboot,
base::Unretained(this), obfuscated_username,
diff --git a/cryptohome/service.h b/cryptohome/service.h
index dd7c5df..ee60822 100644
--- a/cryptohome/service.h
+++ b/cryptohome/service.h
@@ -89,12 +89,12 @@
void PostTask();
// This method is called before processing a task.
- void WillProcessTask(const base::PendingTask& pending_task
#if BASE_VER > 780000
- ,
- bool was_blocked_or_low_priority
+ void WillProcessTask(const base::PendingTask& pending_task,
+ bool was_blocked_or_low_priority) override;
+#else
+ void WillProcessTask(const base::PendingTask& pending_task) override;
#endif
- ) override;
// This method is called after processing a task.
void DidProcessTask(const base::PendingTask& pending_task) override;
@@ -102,7 +102,7 @@
int GetParallelTaskCount() const;
private:
- std::atomic<int> parallel_task_count_;
+ std::atomic<int> parallel_task_count_{0};
};
// Service
