diff --git a/metrics/metrics_daemon.cc b/metrics/metrics_daemon.cc index 7226594cb..acd96c079 100644 --- a/metrics/metrics_daemon.cc +++ b/metrics/metrics_daemon.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -24,7 +25,8 @@ #include #include #include -#include +#include +#include #include "uploader/upload_service.h" using base::FilePath; @@ -43,6 +45,8 @@ namespace { const char kCrashReporterInterface[] = "org.chromium.CrashReporter"; const char kCrashReporterUserCrashSignal[] = "UserCrash"; +const char kCrashReporterMatchRule[] = + "type='signal',interface='%s',path='/',member='%s'"; const int kSecondsPerMinute = 60; const int kMinutesPerHour = 60; @@ -53,7 +57,7 @@ const int kDaysPerWeek = 7; const int kSecondsPerWeek = kSecondsPerDay * kDaysPerWeek; // Interval between calls to UpdateStats(). -const guint kUpdateStatsIntervalMs = 300000; +const uint32_t kUpdateStatsIntervalMs = 300000; const char kKernelCrashDetectedFile[] = "/var/run/kernel-crash-detected"; const char kUncleanShutdownDetectedFile[] = @@ -135,8 +139,7 @@ static const int kMemuseIntervals[] = { }; MetricsDaemon::MetricsDaemon() - : update_stats_timeout_id_(-1), - memuse_final_time_(0), + : memuse_final_time_(0), memuse_interval_index_(0), read_sectors_(0), write_sectors_(0), @@ -147,8 +150,6 @@ MetricsDaemon::MetricsDaemon() latest_cpu_use_ticks_(0) {} MetricsDaemon::~MetricsDaemon() { - if (update_stats_timeout_id_ > -1) - g_source_remove(update_stats_timeout_id_); } double MetricsDaemon::GetActiveTime() { @@ -162,10 +163,7 @@ double MetricsDaemon::GetActiveTime() { } } -void MetricsDaemon::Run(bool run_as_daemon) { - if (run_as_daemon && daemon(0, 0) != 0) - return; - +int MetricsDaemon::Run() { if (CheckSystemCrash(kKernelCrashDetectedFile)) { ProcessKernelCrash(); } @@ -183,7 +181,7 @@ void MetricsDaemon::Run(bool run_as_daemon) { version_cumulative_cpu_use_->Set(0); } - Loop(); + return chromeos::DBusDaemon::Run(); } void MetricsDaemon::RunUploaderTest() { @@ -223,6 +221,7 @@ void MetricsDaemon::Init(bool testing, const string& metrics_file, const string& config_root) { testing_ = testing; + uploader_active_ = uploader_active; config_root_ = config_root; DCHECK(metrics_lib != nullptr); metrics_lib_ = metrics_lib; @@ -276,6 +275,17 @@ void MetricsDaemon::Init(bool testing, vmstats_path_ = vmstats_path; scaling_max_freq_path_ = scaling_max_freq_path; cpuinfo_max_freq_path_ = cpuinfo_max_freq_path; + + // If testing, initialize Stats Reporter without connecting DBus + if (testing_) + StatsReporterInit(); +} + +int MetricsDaemon::OnInit() { + int return_code = chromeos::DBusDaemon::OnInit(); + if (return_code != EX_OK) + return return_code; + StatsReporterInit(); // Start collecting meminfo stats. @@ -283,56 +293,67 @@ void MetricsDaemon::Init(bool testing, memuse_final_time_ = GetActiveTime() + kMemuseIntervals[0]; ScheduleMemuseCallback(kMemuseIntervals[0]); - // Don't setup D-Bus and GLib in test mode. - if (testing) - return; + if (testing_) + return EX_OK; - g_type_init(); - dbus_threads_init_default(); + bus_->AssertOnDBusThread(); + CHECK(bus_->SetUpAsyncOperations()); - DBusError error; - dbus_error_init(&error); + if (bus_->is_connected()) { + const std::string match_rule = + base::StringPrintf(kCrashReporterMatchRule, + kCrashReporterInterface, + kCrashReporterUserCrashSignal); - DBusConnection* connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error); - LOG_IF(FATAL, dbus_error_is_set(&error)) << - "No D-Bus connection: " << SAFE_MESSAGE(error); + bus_->AddFilterFunction(&MetricsDaemon::MessageFilter, this); - dbus_connection_setup_with_g_main(connection, nullptr); + DBusError error; + dbus_error_init(&error); + bus_->AddMatch(match_rule, &error); - vector matches; - matches.push_back( - base::StringPrintf("type='signal',interface='%s',path='/',member='%s'", - kCrashReporterInterface, - kCrashReporterUserCrashSignal)); - - // Registers D-Bus matches for the signals we would like to catch. - for (vector::const_iterator it = matches.begin(); - it != matches.end(); ++it) { - const char* match = it->c_str(); - DLOG(INFO) << "adding dbus match: " << match; - dbus_bus_add_match(connection, match, &error); - LOG_IF(FATAL, dbus_error_is_set(&error)) << - "unable to add a match: " << SAFE_MESSAGE(error); + if (dbus_error_is_set(&error)) { + LOG(ERROR) << "Failed to add match rule \"" << match_rule << "\". Got " + << error.name << ": " << error.message; + return EX_SOFTWARE; + } + } else { + LOG(ERROR) << "DBus isn't connected."; + return EX_UNAVAILABLE; } - // Adds the D-Bus filter routine to be called back whenever one of - // the registered D-Bus matches is successful. The daemon is not - // activated for D-Bus messages that don't match. - CHECK(dbus_connection_add_filter(connection, MessageFilter, this, nullptr)); + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout, + base::Unretained(this)), + base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs)); - update_stats_timeout_id_ = - g_timeout_add(kUpdateStatsIntervalMs, &HandleUpdateStatsTimeout, this); - - if (uploader_active) { + if (uploader_active_) { LOG(INFO) << "uploader enabled"; upload_service_.reset(new UploadService(new SystemProfileCache(), server_)); upload_service_->Init(upload_interval_, metrics_file_); } + + return EX_OK; } -void MetricsDaemon::Loop() { - GMainLoop* loop = g_main_loop_new(nullptr, false); - g_main_loop_run(loop); +void MetricsDaemon::OnShutdown(int* return_code) { + if (!testing_ && bus_->is_connected()) { + const std::string match_rule = + base::StringPrintf(kCrashReporterMatchRule, + kCrashReporterInterface, + kCrashReporterUserCrashSignal); + + bus_->RemoveFilterFunction(&MetricsDaemon::MessageFilter, this); + + DBusError error; + dbus_error_init(&error); + bus_->RemoveMatch(match_rule, &error); + + if (dbus_error_is_set(&error)) { + LOG(ERROR) << "Failed to remove match rule \"" << match_rule << "\". Got " + << error.name << ": " << error.message; + } + } + chromeos::DBusDaemon::OnShutdown(return_code); } // static @@ -481,7 +502,9 @@ void MetricsDaemon::ScheduleStatsCallback(int wait) { if (testing_) { return; } - g_timeout_add_seconds(wait, StatsCallbackStatic, this); + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::StatsCallback, base::Unretained(this)), + base::TimeDelta::FromSeconds(wait)); } bool MetricsDaemon::DiskStatsReadStats(uint64_t* read_sectors, @@ -637,12 +660,6 @@ void MetricsDaemon::SendCpuThrottleMetrics() { SendLinearSample(kMetricScaledCpuFrequencyName, percent, 101, 102); } -// static -gboolean MetricsDaemon::StatsCallbackStatic(void* handle) { - (static_cast(handle))->StatsCallback(); - return false; // one-time callback -} - // Collects disk and vm stats alternating over a short and a long interval. void MetricsDaemon::StatsCallback() { @@ -756,25 +773,31 @@ void MetricsDaemon::ScheduleMeminfoCallback(int wait) { if (testing_) { return; } - g_timeout_add_seconds(wait, MeminfoCallbackStatic, this); + base::TimeDelta waitDelta = base::TimeDelta::FromSeconds(wait); + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this), + base::TimeDelta::FromMilliseconds(kMetricMeminfoInterval)), + waitDelta); } -// static -gboolean MetricsDaemon::MeminfoCallbackStatic(void* handle) { - return (static_cast(handle))->MeminfoCallback(); -} - -bool MetricsDaemon::MeminfoCallback() { +void MetricsDaemon::MeminfoCallback(base::TimeDelta wait) { string meminfo_raw; const FilePath meminfo_path("/proc/meminfo"); if (!base::ReadFileToString(meminfo_path, &meminfo_raw)) { LOG(WARNING) << "cannot read " << meminfo_path.value().c_str(); - return false; + return; } // Make both calls even if the first one fails. bool success = ProcessMeminfo(meminfo_raw); - return ReportZram(base::FilePath(FILE_PATH_LITERAL("/sys/block/zram0"))) && + bool reschedule = + ReportZram(base::FilePath(FILE_PATH_LITERAL("/sys/block/zram0"))) && success; + if (reschedule) { + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::MeminfoCallback, base::Unretained(this), + base::TimeDelta::FromMilliseconds(kMetricMeminfoInterval)), + wait); + } } // static @@ -941,14 +964,9 @@ void MetricsDaemon::ScheduleMemuseCallback(double interval) { if (testing_) { return; } - g_timeout_add_seconds(interval, MemuseCallbackStatic, this); -} - -// static -gboolean MetricsDaemon::MemuseCallbackStatic(void* handle) { - MetricsDaemon* daemon = static_cast(handle); - daemon->MemuseCallback(); - return false; + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::MemuseCallback, base::Unretained(this)), + base::TimeDelta::FromSeconds(interval)); } void MetricsDaemon::MemuseCallback() { @@ -1139,8 +1157,10 @@ void MetricsDaemon::UpdateStats(TimeTicks now_ticks, } } -// static -gboolean MetricsDaemon::HandleUpdateStatsTimeout(gpointer data) { - static_cast(data)->UpdateStats(TimeTicks::Now(), Time::Now()); - return TRUE; +void MetricsDaemon::HandleUpdateStatsTimeout() { + UpdateStats(TimeTicks::Now(), Time::Now()); + base::MessageLoop::current()->PostDelayedTask(FROM_HERE, + base::Bind(&MetricsDaemon::HandleUpdateStatsTimeout, + base::Unretained(this)), + base::TimeDelta::FromMilliseconds(kUpdateStatsIntervalMs)); } diff --git a/metrics/metrics_daemon.h b/metrics/metrics_daemon.h index f4f14307e..397fd2109 100644 --- a/metrics/metrics_daemon.h +++ b/metrics/metrics_daemon.h @@ -7,8 +7,6 @@ #include -#include -#include #include #include #include @@ -16,6 +14,7 @@ #include #include #include +#include #include // for FRIEND_TEST #include "metrics/metrics_library.h" @@ -24,12 +23,12 @@ using chromeos_metrics::PersistentInteger; -class MetricsDaemon { +class MetricsDaemon : public chromeos::DBusDaemon { public: MetricsDaemon(); ~MetricsDaemon(); - // Initializes. + // Initializes metrics class variables. void Init(bool testing, bool uploader_active, MetricsLibraryInterface* metrics_lib, @@ -42,9 +41,14 @@ class MetricsDaemon { const std::string& metrics_file, const std::string& config_root); - // Does all the work. If |run_as_daemon| is true, daemonizes by - // forking. - void Run(bool run_as_daemon); + // Initializes DBus and MessageLoop variables before running the MessageLoop. + int OnInit() override; + + // Clean up data set up in OnInit before shutting down message loop. + void OnShutdown(int* return_code) override; + + // Does all the work. + int Run() override; // Triggers an upload event and exit. (Used to test UploadService) void RunUploaderTest(); @@ -147,9 +151,6 @@ class MetricsDaemon { // Returns the active time since boot (uptime minus sleep time) in seconds. double GetActiveTime(); - // Creates the event loop and enters it. - void Loop(); - // D-Bus filter callback. static DBusHandlerResult MessageFilter(DBusConnection* connection, DBusMessage* message, @@ -227,22 +228,14 @@ class MetricsDaemon { // Parse cumulative vm statistics from a C string. Returns true for success. bool VmStatsParseStats(const char* stats, struct VmstatRecord* record); - // Reports disk and vm statistics (static version for glib). Arguments are a - // glib artifact. - static gboolean StatsCallbackStatic(void* handle); - // Reports disk and vm statistics. void StatsCallback(); // Schedules meminfo collection callback. void ScheduleMeminfoCallback(int wait); - // Reports memory statistics (static version for glib). Argument is a glib - // artifact. - static gboolean MeminfoCallbackStatic(void* handle); - - // Reports memory statistics. Returns false on failure. - bool MeminfoCallback(); + // Reports memory statistics. Reschedules callback on success. + void MeminfoCallback(base::TimeDelta wait); // Parses content of /proc/meminfo and sends fields of interest to UMA. // Returns false on errors. |meminfo_raw| contains the content of @@ -259,9 +252,6 @@ class MetricsDaemon { // Schedule a memory use callback in |interval| seconds. void ScheduleMemuseCallback(double interval); - // Static wrapper for MemuseCallback. Always returns false. - static gboolean MemuseCallbackStatic(void* handle); - // Calls MemuseCallbackWork, and possibly schedules next callback, if enough // active time has passed. Otherwise reschedules itself to simulate active // time callbacks (i.e. wall clock time minus sleep time). @@ -288,7 +278,7 @@ class MetricsDaemon { void UpdateStats(base::TimeTicks now_ticks, base::Time now_wall_time); // Invoked periodically by |update_stats_timeout_id_| to call UpdateStats(). - static gboolean HandleUpdateStatsTimeout(gpointer data); + void HandleUpdateStatsTimeout(); // Reports zram statistics. bool ReportZram(const base::FilePath& zram_dir); @@ -301,6 +291,9 @@ class MetricsDaemon { // Test mode. bool testing_; + // Whether the uploader is enabled or disabled. + bool uploader_active_; + // Root of the configuration files to use. std::string config_root_; @@ -315,16 +308,6 @@ class MetricsDaemon { // The last time that UpdateStats() was called. base::TimeTicks last_update_stats_time_; - // ID of a GLib timeout that repeatedly runs UpdateStats(). - gint update_stats_timeout_id_; - - // Sleep period until the next daily usage aggregation performed by - // the daily use monitor (see ScheduleUseMonitor). - int usemon_interval_; - - // Scheduled daily use monitor source (see ScheduleUseMonitor). - GSource* usemon_source_; - // End time of current memuse stat collection interval. double memuse_final_time_; diff --git a/metrics/metrics_daemon_main.cc b/metrics/metrics_daemon_main.cc index 9322771ee..cc0812d66 100644 --- a/metrics/metrics_daemon_main.cc +++ b/metrics/metrics_daemon_main.cc @@ -73,6 +73,11 @@ int main(int argc, char** argv) { // Also log to stderr when not running as daemon. chromeos::InitLog(chromeos::kLogToSyslog | chromeos::kLogHeader | (FLAGS_daemon ? 0 : chromeos::kLogToStderr)); + + if (FLAGS_daemon && daemon(0, 0) != 0) { + return errno; + } + MetricsLibrary metrics_lib; metrics_lib.Init(); MetricsDaemon daemon; @@ -88,12 +93,10 @@ int main(int argc, char** argv) { FLAGS_metrics_file, FLAGS_config_root); - - base::AtExitManager at_exit_manager; if (FLAGS_uploader_test) { daemon.RunUploaderTest(); return 0; } - daemon.Run(FLAGS_daemon); + daemon.Run(); } diff --git a/metrics/metrics_daemon_test.cc b/metrics/metrics_daemon_test.cc index ed6856a95..abf8e4150 100644 --- a/metrics/metrics_daemon_test.cc +++ b/metrics/metrics_daemon_test.cc @@ -29,6 +29,7 @@ using base::TimeTicks; using std::string; using std::vector; using ::testing::_; +using ::testing::AnyNumber; using ::testing::AtLeast; using ::testing::Return; using ::testing::StrictMock; @@ -227,7 +228,7 @@ TEST_F(MetricsDaemonTest, ReportDailyUse) { TEST_F(MetricsDaemonTest, MessageFilter) { // Ignore calls to SendToUMA. - EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AtLeast(0)); + EXPECT_CALL(metrics_lib_, SendToUMA(_, _, _, _, _)).Times(AnyNumber()); DBusMessage* msg = dbus_message_new(DBUS_MESSAGE_TYPE_METHOD_CALL); DBusHandlerResult res = @@ -265,7 +266,6 @@ TEST_F(MetricsDaemonTest, SendSample) { TEST_F(MetricsDaemonTest, ReportDiskStats) { uint64_t read_sectors_now, write_sectors_now; - CreateFakeDiskStatsFile(kFakeDiskStats1.c_str()); daemon_.DiskStatsReadStats(&read_sectors_now, &write_sectors_now); EXPECT_EQ(read_sectors_now, kFakeReadSectors[1]); @@ -399,8 +399,6 @@ TEST_F(MetricsDaemonTest, SendZramMetrics) { int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); - // Some libchrome calls need this. - base::AtExitManager at_exit_manager; return RUN_ALL_TESTS(); }