diff options
author | Bertrand SIMONNET <bsimonnet@google.com> | 2015-12-08 17:46:00 -0800 |
---|---|---|
committer | Bertrand SIMONNET <bsimonnet@google.com> | 2016-01-12 10:29:40 -0800 |
commit | 9e854564acea5cd41d40d718e8286ca8556dcc0e (patch) | |
tree | 6ca875a41d0ba96e62c32e1032e73a531588c696 | |
parent | ee4be97057e06fe71b5ca8f8c5c3c65841c8815f (diff) | |
download | metricsd-9e854564acea5cd41d40d718e8286ca8556dcc0e.tar.gz |
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
-rw-r--r-- | Android.mk | 2 | ||||
-rw-r--r-- | metricsd_main.cc | 20 | ||||
-rw-r--r-- | uploader/bn_metricsd_impl.cc | 12 | ||||
-rw-r--r-- | uploader/bn_metricsd_impl.h | 3 | ||||
-rw-r--r-- | uploader/metricsd_service_runner.cc | 60 | ||||
-rw-r--r-- | uploader/metricsd_service_runner.h | 47 | ||||
-rw-r--r-- | uploader/upload_service.cc | 21 | ||||
-rw-r--r-- | uploader/upload_service.h | 11 | ||||
-rw-r--r-- | uploader/upload_service_test.cc | 13 |
9 files changed, 141 insertions, 48 deletions
@@ -38,6 +38,7 @@ metricsd_common := \ 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 @@ metrics_collector_static_libraries := libmetricscollectorservice metricsd_shared_libraries := \ libbinder \ libbrillo \ + libbrillo-binder \ libbrillo-http \ libchrome \ libprotobuf-cpp-lite \ diff --git a/metricsd_main.cc b/metricsd_main.cc index f460268..ae441d0 100644 --- a/metricsd_main.cc +++ b/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 @@ int main(int argc, char** argv) { 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::StatisticsRecorder::Initialize(); - - // Create and start the binder thread. - BnMetricsdImpl binder_service(counters); - std::thread binder_thread(&BnMetricsdImpl::Run, &binder_service); + base::FilePath(FLAGS_shared_directory)); - upload_service.Run(); + return upload_service.Run(); } diff --git a/uploader/bn_metricsd_impl.cc b/uploader/bn_metricsd_impl.cc index 2cbc2da..219ed60 100644 --- a/uploader/bn_metricsd_impl.cc +++ b/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 @@ BnMetricsdImpl::BnMetricsdImpl(const std::shared_ptr<CrashCounters>& counters) 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/uploader/bn_metricsd_impl.h b/uploader/bn_metricsd_impl.h index 016ccb6..bf47e80 100644 --- a/uploader/bn_metricsd_impl.h +++ b/uploader/bn_metricsd_impl.h @@ -25,9 +25,6 @@ class BnMetricsdImpl : public android::brillo::metrics::BnMetricsd { 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/uploader/metricsd_service_runner.cc b/uploader/metricsd_service_runner.cc new file mode 100644 index 0000000..2834977 --- /dev/null +++ b/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/uploader/metricsd_service_runner.h b/uploader/metricsd_service_runner.h new file mode 100644 index 0000000..1715de0 --- /dev/null +++ b/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/uploader/upload_service.cc b/uploader/upload_service.cc index 2fb30c3..0d7aacc 100644 --- a/uploader/upload_service.cc +++ b/uploader/upload_service.cc @@ -43,19 +43,25 @@ const int UploadService::kMaxFailedUpload = 10; 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 @@ int UploadService::OnInit() { 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/uploader/upload_service.h b/uploader/upload_service.h index 420653e..b84179f 100644 --- a/uploader/upload_service.h +++ b/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 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { 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 @@ class UploadService : public base::HistogramFlattener, public brillo::Daemon { base::TimeDelta upload_interval_; + MetricsdServiceRunner metricsd_service_runner_; + base::FilePath consent_file_; base::FilePath staged_log_path_; diff --git a/uploader/upload_service_test.cc b/uploader/upload_service_test.cc index ec507e8..bd5b39a 100644 --- a/uploader/upload_service_test.cc +++ b/uploader/upload_service_test.cc @@ -53,10 +53,10 @@ class UploadServiceTest : public testing::Test { 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, EmptyLogsAreNotSent) { } 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) { |