From 6e6d942d82bab86bcebc67620176490a95777196 Mon Sep 17 00:00:00 2001 From: Anton Date: Mon, 22 Apr 2024 16:32:47 -0700 Subject: [PATCH] Added persistent setting for logtrace killswitch. (#3005) Added persistent setting for logtrace killswitch. When kPersistentSettingLogtraceEnable = false, then all logtrace methods are noop. b/327680765 Change-Id: I78fecc80d64c8fe1b3d1a9caf8fd89608471ec14 (cherry picked from commit 77b63853742a575e5a65f0c15d470ef226ff2566) --- cobalt/watchdog/watchdog.cc | 47 ++++++++++++----- cobalt/watchdog/watchdog.h | 27 ++++++++++ cobalt/watchdog/watchdog_test.cc | 89 +++++++++++++++++++++++++++++++- 3 files changed, 148 insertions(+), 15 deletions(-) diff --git a/cobalt/watchdog/watchdog.cc b/cobalt/watchdog/watchdog.cc index f2c72b84a43a..0d5daf8296dd 100644 --- a/cobalt/watchdog/watchdog.cc +++ b/cobalt/watchdog/watchdog.cc @@ -53,19 +53,6 @@ const int kWatchdogMaxPingInfoLength = 1024; // The maximum number of milliseconds old of an unfetched Watchdog violation. const int64_t kWatchdogMaxViolationsAge = 86400000; -// Persistent setting name and default setting for the boolean that controls -// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a -// stub except that persistent settings can still be get/set. Requires a -// restart to take effect. -const char kPersistentSettingWatchdogEnable[] = - "kPersistentSettingWatchdogEnable"; -const bool kDefaultSettingWatchdogEnable = true; -// Persistent setting name and default setting for the boolean that controls -// whether or not a Watchdog violation will trigger a crash. -const char kPersistentSettingWatchdogCrash[] = - "kPersistentSettingWatchdogCrash"; -const bool kDefaultSettingWatchdogCrash = false; - } // namespace bool Watchdog::Initialize( @@ -80,6 +67,7 @@ bool Watchdog::InitializeCustom( std::string watchdog_file_name, int64_t watchdog_monitor_frequency) { persistent_settings_ = persistent_settings; is_disabled_ = !GetPersistentSettingWatchdogEnable(); + is_logtrace_disabled_ = !GetPersistentSettingLogtraceEnable(); if (is_disabled_) return true; @@ -805,14 +793,45 @@ void Watchdog::SetPersistentSettingWatchdogCrash(bool can_trigger_crash) { } bool Watchdog::LogEvent(const std::string& event) { + if (is_logtrace_disabled_) { + return true; + } + return instrumentation_log_.LogEvent(event); } std::vector Watchdog::GetLogTrace() { + if (is_logtrace_disabled_) { + return {}; + } + return instrumentation_log_.GetLogTrace(); } -void Watchdog::ClearLog() { instrumentation_log_.ClearLog(); } +void Watchdog::ClearLog() { + if (is_logtrace_disabled_) { + return; + } + + instrumentation_log_.ClearLog(); +} + +bool Watchdog::GetPersistentSettingLogtraceEnable() { + if (!persistent_settings_) return kDefaultSettingLogtraceEnable; + + // Gets the boolean that controls whether or not LogTrace is enabled. + return persistent_settings_->GetPersistentSettingAsBool( + kPersistentSettingLogtraceEnable, kDefaultSettingLogtraceEnable); +} + +void Watchdog::SetPersistentSettingLogtraceEnable(bool enable_logtrace) { + if (!persistent_settings_) return; + + // Sets the boolean that controls whether or not LogTrace is enabled. + persistent_settings_->SetPersistentSetting( + kPersistentSettingLogtraceEnable, + std::make_unique(enable_logtrace)); +} #if defined(_DEBUG) // Sleeps threads for Watchdog debugging. diff --git a/cobalt/watchdog/watchdog.h b/cobalt/watchdog/watchdog.h index f967957f793d..412afef74841 100644 --- a/cobalt/watchdog/watchdog.h +++ b/cobalt/watchdog/watchdog.h @@ -33,6 +33,28 @@ namespace cobalt { namespace watchdog { +// Persistent setting name and default setting for the boolean that controls +// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a +// stub except that persistent settings can still be get/set. Requires a +// restart to take effect. +constexpr char kPersistentSettingWatchdogEnable[] = + "kPersistentSettingWatchdogEnable"; +constexpr bool kDefaultSettingWatchdogEnable = true; + +// Persistent setting name and default setting for the boolean that controls +// whether or not a Watchdog violation will trigger a crash. +constexpr char kPersistentSettingWatchdogCrash[] = + "kPersistentSettingWatchdogCrash"; +constexpr bool kDefaultSettingWatchdogCrash = false; + +// Persistent setting name and default setting for the boolean that controls +// whether or not LogTrace API is enabled. When disabled, all LogTrace methods +// behave like a stub except for persistent settings itself. Requires a +// restart to take effect. +constexpr char kPersistentSettingLogtraceEnable[] = + "kPersistentSettingLogtraceEnable"; +constexpr bool kDefaultSettingLogtraceEnable = true; + // Client to monitor typedef struct Client { std::string name; @@ -112,6 +134,8 @@ class Watchdog : public Singleton { bool LogEvent(const std::string& event); std::vector GetLogTrace(); void ClearLog(); + bool GetPersistentSettingLogtraceEnable(); + void SetPersistentSettingLogtraceEnable(bool enable_logtrace); #if defined(_DEBUG) // Sleeps threads based off of environment variables for Watchdog debugging. @@ -181,6 +205,9 @@ class Watchdog : public Singleton { int64_t watchdog_monitor_frequency_; // Captures string events emitted from Kabuki via logEvent() h5vcc API. InstrumentationLog instrumentation_log_; + // Flag to disable LogTrace API. When disabled, all LogTrace methods behave + // like a stub except that the flag itself can still be get/set. + bool is_logtrace_disabled_; #if defined(_DEBUG) starboard::Mutex delay_mutex_; diff --git a/cobalt/watchdog/watchdog_test.cc b/cobalt/watchdog/watchdog_test.cc index ee14ee6f7ff8..95e3fa2d4567 100644 --- a/cobalt/watchdog/watchdog_test.cc +++ b/cobalt/watchdog/watchdog_test.cc @@ -22,12 +22,15 @@ #include "base/json/json_reader.h" #include "base/json/json_writer.h" +#include "base/test/task_environment.h" #include "starboard/common/file.h" #include "testing/gtest/include/gtest/gtest.h" namespace cobalt { namespace watchdog { +using persistent_storage::PersistentSettings; + namespace { const char kWatchdogViolationsJson[] = "watchdog_test.json"; @@ -38,7 +41,9 @@ const int64_t kWatchdogSleepDuration = kWatchdogMonitorFrequency * 4; class WatchdogTest : public testing::Test { protected: - WatchdogTest() {} + WatchdogTest() + : task_environment_( + base::test::TaskEnvironment::MainThreadType::DEFAULT) {} void SetUp() final { watchdog_ = new watchdog::Watchdog(); @@ -51,6 +56,8 @@ class WatchdogTest : public testing::Test { watchdog_->Uninitialize(); delete watchdog_; watchdog_ = nullptr; + + DeletePersistentSettingsFile(); } base::Value CreateDummyViolationDict(std::string desc, int begin, int end) { @@ -82,7 +89,23 @@ class WatchdogTest : public testing::Test { return violation.Clone(); } + void DeletePersistentSettingsFile() { + std::vector storage_dir(kSbFileMaxPath + 1, 0); + SbSystemGetPath(kSbSystemPathCacheDirectory, storage_dir.data(), + kSbFileMaxPath); + std::string path = + std::string(storage_dir.data()) + kSbFileSepString + kSettingsFileName; + + starboard::SbFileDeleteRecursive(path.c_str(), true); + } + + const std::string kSettingsFileName = "test-settings.json"; + watchdog::Watchdog* watchdog_; + base::test::TaskEnvironment task_environment_; + base::WaitableEvent task_done_ = { + base::WaitableEvent::ResetPolicy::MANUAL, + base::WaitableEvent::InitialState::NOT_SIGNALED}; }; TEST_F(WatchdogTest, RedundantRegistersShouldFail) { @@ -719,5 +742,69 @@ TEST_F(WatchdogTest, ViolationContainsEmptyLogTrace) { ASSERT_EQ(logTrace->size(), 0); } +TEST_F(WatchdogTest, WatchdogMethodsAreNoopWhenWatchdogIsDisabled) { + // init and destroy existing watchdog to re-initialize it later + watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted, + kWatchdogMonitorFrequency); + TearDown(); + + // PersistentSettings doesn't have interface so it's not mockable + auto persistent_settings = + std::make_unique(kSettingsFileName); + persistent_settings->ValidatePersistentSettings(); + + base::OnceClosure closure = base::BindOnce( + [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_); + persistent_settings->SetPersistentSetting( + kPersistentSettingWatchdogEnable, std::make_unique(false), + std::move(closure), true); + task_done_.Wait(); + + watchdog_ = new watchdog::Watchdog(); + watchdog_->InitializeCustom(persistent_settings.get(), + std::string(kWatchdogViolationsJson), + kWatchdogMonitorFrequency); + + ASSERT_TRUE(watchdog_->Register("test-name", "test-desc", + base::kApplicationStateStarted, + kWatchdogMonitorFrequency)); + ASSERT_TRUE(watchdog_->Ping("test-name")); + ASSERT_TRUE(watchdog_->PingByClient(nullptr)); + + usleep(kWatchdogSleepDuration); + + ASSERT_EQ(watchdog_->GetWatchdogViolations(), ""); + ASSERT_TRUE(watchdog_->Unregister("test-name")); + ASSERT_TRUE(watchdog_->Unregister("")); +} + +TEST_F(WatchdogTest, LogtraceMethodsAreNoopWhenLogtraceIsDisabled) { + // init and destroy existing watchdog to re-initialize it later + watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted, + kWatchdogMonitorFrequency); + TearDown(); + + // PersistentSettings doesn't have interface so it's not mockable + auto persistent_settings = + std::make_unique(kSettingsFileName); + persistent_settings->ValidatePersistentSettings(); + + base::OnceClosure closure = base::BindOnce( + [](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_); + persistent_settings->SetPersistentSetting( + kPersistentSettingLogtraceEnable, std::make_unique(false), + std::move(closure), true); + task_done_.Wait(); + + watchdog_ = new watchdog::Watchdog(); + watchdog_->InitializeCustom(persistent_settings.get(), + std::string(kWatchdogViolationsJson), + kWatchdogMonitorFrequency); + + ASSERT_TRUE(watchdog_->LogEvent("foo")); + ASSERT_EQ(watchdog_->GetLogTrace().size(), 0); + ASSERT_NO_FATAL_FAILURE(watchdog_->ClearLog()); +} + } // namespace watchdog } // namespace cobalt