metricsd: Cleanup on TERM signal.
metricsd should shutdown cleanly when receiving a TERM signal:
* stop the binder watcher.
* wait for the thread to quit.
* Exit cleanly.
Note: This is not directly used as we don't send SIGTERM on shutdown or
when stopping services yet.
Bug: 25670584
Change-Id: I878d1e67474c72d24790f3540470e37a23112a95
diff --git a/metricsd/Android.mk b/metricsd/Android.mk
index 7381703..ed3fcbb 100644
--- a/metricsd/Android.mk
+++ b/metricsd/Android.mk
@@ -38,6 +38,7 @@
uploader/metrics_hashes.cc \
uploader/metrics_log_base.cc \
uploader/metrics_log.cc \
+ uploader/metricsd_service_runner.cc \
uploader/sender_http.cc \
uploader/system_profile_cache.cc \
uploader/upload_service.cc
@@ -84,6 +85,7 @@
metricsd_shared_libraries := \
libbinder \
libbrillo \
+ libbrillo-binder \
libbrillo-http \
libchrome \
libprotobuf-cpp-lite \
diff --git a/metricsd/metricsd_main.cc b/metricsd/metricsd_main.cc
index f460268..ae441d0 100644
--- a/metricsd/metricsd_main.cc
+++ b/metricsd/metricsd_main.cc
@@ -14,21 +14,15 @@
* limitations under the License.
*/
-#include <thread>
-
-#include <base/at_exit.h>
#include <base/command_line.h>
#include <base/files/file_path.h>
#include <base/logging.h>
-#include <base/metrics/statistics_recorder.h>
-#include <base/strings/string_util.h>
#include <base/time/time.h>
#include <brillo/flag_helper.h>
#include <brillo/syslog_logging.h>
#include "constants.h"
-#include "uploader/bn_metricsd_impl.h"
-#include "uploader/crash_counters.h"
+#include "uploader/metricsd_service_runner.h"
#include "uploader/upload_service.h"
int main(int argc, char** argv) {
@@ -76,18 +70,10 @@
return errno;
}
- std::shared_ptr<CrashCounters> counters(new CrashCounters);
-
UploadService upload_service(
FLAGS_server, base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
base::FilePath(FLAGS_private_directory),
- base::FilePath(FLAGS_shared_directory), counters);
+ base::FilePath(FLAGS_shared_directory));
- base::StatisticsRecorder::Initialize();
-
- // Create and start the binder thread.
- BnMetricsdImpl binder_service(counters);
- std::thread binder_thread(&BnMetricsdImpl::Run, &binder_service);
-
- upload_service.Run();
+ return upload_service.Run();
}
diff --git a/metricsd/uploader/bn_metricsd_impl.cc b/metricsd/uploader/bn_metricsd_impl.cc
index 2cbc2da..219ed60 100644
--- a/metricsd/uploader/bn_metricsd_impl.cc
+++ b/metricsd/uploader/bn_metricsd_impl.cc
@@ -19,8 +19,6 @@
#include <base/metrics/histogram.h>
#include <base/metrics/sparse_histogram.h>
#include <base/metrics/statistics_recorder.h>
-#include <binder/IPCThreadState.h>
-#include <binder/IServiceManager.h>
#include <utils/Errors.h>
#include <utils/String16.h>
#include <utils/String8.h>
@@ -37,16 +35,6 @@
CHECK(counters_) << "Invalid counters argument to constructor";
}
-void BnMetricsdImpl::Run() {
- android::status_t status =
- android::defaultServiceManager()->addService(getInterfaceDescriptor(),
- this);
- CHECK(status == android::OK) << "Metricsd service registration failed";
- android::ProcessState::self()->setThreadPoolMaxThreadCount(0);
- android::IPCThreadState::self()->disableBackgroundScheduling(true);
- android::IPCThreadState::self()->joinThreadPool();
-}
-
Status BnMetricsdImpl::recordHistogram(
const String16& name, int sample, int min, int max, int nbuckets) {
base::HistogramBase* histogram = base::Histogram::FactoryGet(
diff --git a/metricsd/uploader/bn_metricsd_impl.h b/metricsd/uploader/bn_metricsd_impl.h
index 016ccb6..bf47e80 100644
--- a/metricsd/uploader/bn_metricsd_impl.h
+++ b/metricsd/uploader/bn_metricsd_impl.h
@@ -25,9 +25,6 @@
explicit BnMetricsdImpl(const std::shared_ptr<CrashCounters>& counters);
virtual ~BnMetricsdImpl() = default;
- // Starts the binder main loop.
- void Run();
-
// Records a histogram.
android::binder::Status recordHistogram(const android::String16& name,
int sample,
diff --git a/metricsd/uploader/metricsd_service_runner.cc b/metricsd/uploader/metricsd_service_runner.cc
new file mode 100644
index 0000000..2834977
--- /dev/null
+++ b/metricsd/uploader/metricsd_service_runner.cc
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "uploader/metricsd_service_runner.h"
+
+#include <thread>
+
+#include <binder/IServiceManager.h>
+#include <brillo/binder_watcher.h>
+#include <utils/Errors.h>
+
+#include "uploader/bn_metricsd_impl.h"
+
+MetricsdServiceRunner::MetricsdServiceRunner(
+ std::shared_ptr<CrashCounters> counters)
+ : counters_(counters) {}
+
+void MetricsdServiceRunner::Start() {
+ thread_.reset(new std::thread(&MetricsdServiceRunner::Run, this));
+}
+
+void MetricsdServiceRunner::Run() {
+ android::sp<BnMetricsdImpl> metrics_service(new BnMetricsdImpl(counters_));
+
+ android::status_t status = android::defaultServiceManager()->addService(
+ metrics_service->getInterfaceDescriptor(), metrics_service);
+ CHECK(status == android::OK) << "Metricsd service registration failed";
+
+ message_loop_for_io_.reset(new base::MessageLoopForIO);
+
+ brillo::BinderWatcher watcher;
+ CHECK(watcher.Init()) << "failed to initialize the binder file descriptor "
+ << "watcher";
+
+ message_loop_for_io_->Run();
+
+ // Delete the message loop here as it needs to be deconstructed in the thread
+ // it is attached to.
+ message_loop_for_io_.reset();
+}
+
+void MetricsdServiceRunner::Stop() {
+ message_loop_for_io_->PostTask(FROM_HERE,
+ message_loop_for_io_->QuitClosure());
+
+ thread_->join();
+}
diff --git a/metricsd/uploader/metricsd_service_runner.h b/metricsd/uploader/metricsd_service_runner.h
new file mode 100644
index 0000000..1715de0
--- /dev/null
+++ b/metricsd/uploader/metricsd_service_runner.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef METRICS_UPLOADER_METRISCD_SERVICE_RUNNER_H_
+#define METRICS_UPLOADER_METRISCD_SERVICE_RUNNER_H_
+
+#include <memory>
+#include <thread>
+
+#include <base/message_loop/message_loop.h>
+
+#include "uploader/crash_counters.h"
+
+class MetricsdServiceRunner {
+ public:
+ MetricsdServiceRunner(std::shared_ptr<CrashCounters> counters);
+
+ // Start the Metricsd Binder service in a new thread.
+ void Start();
+
+ // Stop the Metricsd service and wait for its thread to exit.
+ void Stop();
+
+ private:
+ // Creates and run the main loop for metricsd's Binder service.
+ void Run();
+
+ std::unique_ptr<base::MessageLoopForIO> message_loop_for_io_;
+
+ std::unique_ptr<std::thread> thread_;
+ std::shared_ptr<CrashCounters> counters_;
+};
+
+#endif // METRICS_UPLOADER_METRISCD_SERVICE_RUNNER_H_
diff --git a/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc
index 2fb30c3..0d7aacc 100644
--- a/metricsd/uploader/upload_service.cc
+++ b/metricsd/uploader/upload_service.cc
@@ -43,19 +43,25 @@
UploadService::UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
const base::FilePath& private_metrics_directory,
- const base::FilePath& shared_metrics_directory,
- const std::shared_ptr<CrashCounters> counters)
- : histogram_snapshot_manager_(this),
+ const base::FilePath& shared_metrics_directory)
+ : brillo::Daemon(),
+ histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
failed_upload_count_(metrics::kFailedUploadCountName,
private_metrics_directory),
- counters_(counters),
- upload_interval_(upload_interval) {
+ counters_(new CrashCounters),
+ upload_interval_(upload_interval),
+ metricsd_service_runner_(counters_) {
staged_log_path_ = private_metrics_directory.Append(metrics::kStagedLogName);
consent_file_ = shared_metrics_directory.Append(metrics::kConsentFileName);
}
int UploadService::OnInit() {
+ brillo::Daemon::OnInit();
+
+ base::StatisticsRecorder::Initialize();
+ metricsd_service_runner_.Start();
+
system_profile_setter_.reset(new SystemProfileCache());
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
@@ -63,9 +69,14 @@
base::Unretained(this),
upload_interval_),
upload_interval_);
+
return EX_OK;
}
+void UploadService::OnShutdown(int* exit_code) {
+ metricsd_service_runner_.Stop();
+}
+
void UploadService::InitForTest(SystemProfileSetter* setter) {
system_profile_setter_.reset(setter);
}
diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h
index 420653e..b84179f 100644
--- a/metricsd/uploader/upload_service.h
+++ b/metricsd/uploader/upload_service.h
@@ -28,6 +28,7 @@
#include "persistent_integer.h"
#include "uploader/crash_counters.h"
#include "uploader/metrics_log.h"
+#include "uploader/metricsd_service_runner.h"
#include "uploader/proto/chrome_user_metrics_extension.pb.h"
#include "uploader/sender.h"
#include "uploader/system_profile_cache.h"
@@ -66,11 +67,13 @@
UploadService(const std::string& server,
const base::TimeDelta& upload_interval,
const base::FilePath& private_metrics_directory,
- const base::FilePath& shared_metrics_directory,
- const std::shared_ptr<CrashCounters> counters);
+ const base::FilePath& shared_metrics_directory);
// Initializes the upload service.
- int OnInit();
+ int OnInit() override;
+
+ // Cleans up the internal state before exiting.
+ void OnShutdown(int* exit_code) override;
// Starts a new log. The log needs to be regenerated after each successful
// launch as it is destroyed when staging the log.
@@ -154,6 +157,8 @@
base::TimeDelta upload_interval_;
+ MetricsdServiceRunner metricsd_service_runner_;
+
base::FilePath consent_file_;
base::FilePath staged_log_path_;
diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc
index ec507e8..bd5b39a 100644
--- a/metricsd/uploader/upload_service_test.cc
+++ b/metricsd/uploader/upload_service_test.cc
@@ -53,10 +53,10 @@
ASSERT_EQ(0, base::WriteFile(shared_dir.Append(metrics::kConsentFileName),
"", 0));
- counters_.reset(new CrashCounters);
- upload_service_.reset(new UploadService("", base::TimeDelta(), private_dir,
- shared_dir, counters_));
+ upload_service_.reset(
+ new UploadService("", base::TimeDelta(), private_dir, shared_dir));
+ counters_ = upload_service_->counters_;
upload_service_->sender_.reset(new SenderMock);
upload_service_->InitForTest(new MockSystemProfileSetter);
@@ -149,12 +149,9 @@
}
TEST_F(UploadServiceTest, LogEmptyByDefault) {
- UploadService upload_service("", base::TimeDelta(), dir_.path(), dir_.path(),
- std::make_shared<CrashCounters>());
-
- // current_log_ should be initialized later as it needs AtExitManager to exit
+ // current_log_ should be initialized later as it needs AtExitManager to exist
// in order to gather system information from SysInfo.
- EXPECT_FALSE(upload_service.current_log_);
+ EXPECT_FALSE(upload_service_->current_log_);
}
TEST_F(UploadServiceTest, CanSendMultipleTimes) {