From e449b0c1855b9a8d967d8926bc295c1c1c16922c Mon Sep 17 00:00:00 2001 From: Ruchir Rastogi Date: Mon, 10 Feb 2020 17:40:09 -0800 Subject: [PATCH] Move statsd (and tests) to libbinder_ndk Major changes include: - Removing unused permission checks within StatsService. These include ENFORCE_DUMP_AND_USAGE_STATS, checkDumpAndUsageStats, kOpUsage, and kPermissionUsage. - Converting from sp to shared_ptr - Using libbinder_ndk functions instead of libbinder functions (e.g. for installing death recipients, getting calling uids, etc.) - New death recipients were added in StatsService, ConfigManager, and SubscriberReporter. - Using a unique token (timestamp) to identify shell subscribers instead of IResultReceiver because IResultReceiver is not exposed by libbinder_ndk. Currently, statsd cannot detect if perfd dies; we will fix that later. Bug: 145232107 Bug: 148609603 Test: m statsd Test: m statsd_test Test: bit stastd_test:* Test: atest GtsStatsdHostTestCases Change-Id: Ia1fda7280c22320bc4ebc8371acaadbe8eabcbd2 --- cmds/statsd/Android.bp | 4 +- cmds/statsd/src/HashableDimensionKey.cpp | 3 +- cmds/statsd/src/HashableDimensionKey.h | 4 +- cmds/statsd/src/StatsService.cpp | 325 ++++++++---------- cmds/statsd/src/StatsService.h | 70 ++-- cmds/statsd/src/anomaly/AlarmMonitor.cpp | 14 +- cmds/statsd/src/anomaly/AlarmMonitor.h | 19 +- cmds/statsd/src/anomaly/AlarmTracker.h | 3 - cmds/statsd/src/config/ConfigManager.cpp | 99 ++++-- cmds/statsd/src/config/ConfigManager.h | 22 +- .../src/external/PullResultReceiver.cpp | 7 +- cmds/statsd/src/external/PullResultReceiver.h | 16 +- .../src/external/StatsCallbackPuller.cpp | 22 +- .../statsd/src/external/StatsCallbackPuller.h | 12 +- cmds/statsd/src/external/StatsPuller.h | 8 +- .../src/external/StatsPullerManager.cpp | 18 +- cmds/statsd/src/external/StatsPullerManager.h | 16 +- cmds/statsd/src/logd/LogEvent.cpp | 8 +- cmds/statsd/src/main.cpp | 35 +- cmds/statsd/src/packages/UidMap.h | 3 +- cmds/statsd/src/shell/ShellSubscriber.cpp | 199 +++++------ cmds/statsd/src/shell/ShellSubscriber.h | 18 +- cmds/statsd/src/stats_log_util.cpp | 9 +- cmds/statsd/src/statscompanion_util.cpp | 20 +- cmds/statsd/src/statscompanion_util.h | 7 +- .../src/subscriber/SubscriberReporter.cpp | 72 ++-- .../src/subscriber/SubscriberReporter.h | 28 +- cmds/statsd/tests/AlarmMonitor_test.cpp | 6 +- cmds/statsd/tests/StatsLogProcessor_test.cpp | 20 +- cmds/statsd/tests/StatsService_test.cpp | 34 +- .../tests/anomaly/AlarmTracker_test.cpp | 7 +- .../tests/e2e/GaugeMetric_e2e_pull_test.cpp | 18 +- .../tests/e2e/PartialBucket_e2e_test.cpp | 116 +++---- .../tests/e2e/ValueMetric_pull_e2e_test.cpp | 9 +- .../external/StatsCallbackPuller_test.cpp | 37 +- .../tests/shell/ShellSubscriber_test.cpp | 29 +- cmds/statsd/tests/statsd_test_util.cpp | 27 +- cmds/statsd/tests/statsd_test_util.h | 16 +- 38 files changed, 711 insertions(+), 669 deletions(-) diff --git a/cmds/statsd/Android.bp b/cmds/statsd/Android.bp index 956fd29205cbe..2ee055da4bf11 100644 --- a/cmds/statsd/Android.bp +++ b/cmds/statsd/Android.bp @@ -123,11 +123,11 @@ cc_defaults { "libutils", ], shared_libs: [ - "libbinder", + "libbinder_ndk", "libincident", "liblog", "libstatssocket", - "statsd-aidl-cpp", + "statsd-aidl-ndk_platform", ], } diff --git a/cmds/statsd/src/HashableDimensionKey.cpp b/cmds/statsd/src/HashableDimensionKey.cpp index 6b9d0e4fdac06..d95d59454dd88 100644 --- a/cmds/statsd/src/HashableDimensionKey.cpp +++ b/cmds/statsd/src/HashableDimensionKey.cpp @@ -25,6 +25,7 @@ namespace statsd { using std::string; using std::vector; +using android::base::StringPrintf; // These constants must be kept in sync with those in StatsDimensionsValue.java const static int STATS_DIMENSIONS_VALUE_STRING_TYPE = 2; @@ -73,7 +74,7 @@ static void populateStatsDimensionsValueParcelChildren(StatsDimensionsValueParce break; case STRING: childParcel.valueType = STATS_DIMENSIONS_VALUE_STRING_TYPE; - childParcel.stringValue = String16(dim.mValue.str_value.c_str()); + childParcel.stringValue = dim.mValue.str_value; break; default: ALOGE("Encountered FieldValue with unsupported value type."); diff --git a/cmds/statsd/src/HashableDimensionKey.h b/cmds/statsd/src/HashableDimensionKey.h index 4adcf967555e5..a766bbae00d39 100644 --- a/cmds/statsd/src/HashableDimensionKey.h +++ b/cmds/statsd/src/HashableDimensionKey.h @@ -16,7 +16,7 @@ #pragma once -#include +#include #include #include #include "android-base/stringprintf.h" @@ -27,7 +27,7 @@ namespace android { namespace os { namespace statsd { -using android::base::StringPrintf; +using ::aidl::android::os::StatsDimensionsValueParcel; struct Metric2Condition { int64_t conditionId; diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp index 168833fc8c8ba..7087c68ecc9fd 100644 --- a/cmds/statsd/src/StatsService.cpp +++ b/cmds/statsd/src/StatsService.cpp @@ -28,8 +28,6 @@ #include #include -#include -#include #include #include #include @@ -47,85 +45,42 @@ using android::base::StringPrintf; using android::util::FIELD_COUNT_REPEATED; using android::util::FIELD_TYPE_MESSAGE; +using Status = ::ndk::ScopedAStatus; + namespace android { namespace os { namespace statsd { constexpr const char* kPermissionDump = "android.permission.DUMP"; -constexpr const char* kPermissionUsage = "android.permission.PACKAGE_USAGE_STATS"; - -constexpr const char* kOpUsage = "android:get_usage_stats"; #define STATS_SERVICE_DIR "/data/misc/stats-service" // for StatsDataDumpProto const int FIELD_ID_REPORTS_LIST = 1; -static binder::Status ok() { - return binder::Status::ok(); -} - -static binder::Status exception(uint32_t code, const std::string& msg) { +static Status exception(int32_t code, const std::string& msg) { ALOGE("%s (%d)", msg.c_str(), code); - return binder::Status::fromExceptionCode(code, String8(msg.c_str())); + return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(code, msg.c_str())); } static bool checkPermission(const char* permission) { - pid_t pid = IPCThreadState::self()->getCallingPid(); - uid_t uid = IPCThreadState::self()->getCallingUid(); + pid_t pid = AIBinder_getCallingPid(); + uid_t uid = AIBinder_getCallingUid(); return checkPermissionForIds(permission, pid, uid); } -binder::Status checkUid(uid_t expectedUid) { - uid_t uid = IPCThreadState::self()->getCallingUid(); +Status checkUid(uid_t expectedUid) { + uid_t uid = AIBinder_getCallingUid(); if (uid == expectedUid || uid == AID_ROOT) { - return ok(); + return Status::ok(); } else { - return exception(binder::Status::EX_SECURITY, - StringPrintf("UID %d is not expected UID %d", uid, expectedUid)); - } -} - -binder::Status checkDumpAndUsageStats(const String16& packageName) { - pid_t pid = IPCThreadState::self()->getCallingPid(); - uid_t uid = IPCThreadState::self()->getCallingUid(); - - // Root, system, and shell always have access - if (uid == AID_ROOT || uid == AID_SYSTEM || uid == AID_SHELL) { - return ok(); - } - - // Caller must be granted these permissions - if (!checkPermission(kPermissionDump)) { - return exception(binder::Status::EX_SECURITY, - StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, kPermissionDump)); - } - if (!checkPermission(kPermissionUsage)) { - return exception(binder::Status::EX_SECURITY, - StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, kPermissionUsage)); - } - - // Caller must also have usage stats op granted - PermissionController pc; - switch (pc.noteOp(String16(kOpUsage), uid, packageName)) { - case PermissionController::MODE_ALLOWED: - case PermissionController::MODE_DEFAULT: - return ok(); - default: - return exception(binder::Status::EX_SECURITY, - StringPrintf("UID %d / PID %d lacks app-op %s", uid, pid, kOpUsage)); + return exception(EX_SECURITY, + StringPrintf("UID %d is not expected UID %d", uid, expectedUid)); } } #define ENFORCE_UID(uid) { \ - binder::Status status = checkUid((uid)); \ - if (!status.isOk()) { \ - return status; \ - } \ -} - -#define ENFORCE_DUMP_AND_USAGE_STATS(packageName) { \ - binder::Status status = checkDumpAndUsageStats(packageName); \ + Status status = checkUid((uid)); \ if (!status.isOk()) { \ return status; \ } \ @@ -134,13 +89,13 @@ binder::Status checkDumpAndUsageStats(const String16& packageName) { StatsService::StatsService(const sp& handlerLooper, shared_ptr queue) : mAnomalyAlarmMonitor(new AlarmMonitor( MIN_DIFF_TO_UPDATE_REGISTERED_ALARM_SECS, - [](const sp& sc, int64_t timeMillis) { + [](const shared_ptr& sc, int64_t timeMillis) { if (sc != nullptr) { sc->setAnomalyAlarm(timeMillis); StatsdStats::getInstance().noteRegisteredAnomalyAlarmChanged(); } }, - [](const sp& sc) { + [](const shared_ptr& sc) { if (sc != nullptr) { sc->cancelAnomalyAlarm(); StatsdStats::getInstance().noteRegisteredAnomalyAlarmChanged(); @@ -148,19 +103,21 @@ StatsService::StatsService(const sp& handlerLooper, shared_ptr& sc, int64_t timeMillis) { + [](const shared_ptr& sc, int64_t timeMillis) { if (sc != nullptr) { sc->setAlarmForSubscriberTriggering(timeMillis); StatsdStats::getInstance().noteRegisteredPeriodicAlarmChanged(); } }, - [](const sp& sc) { + [](const shared_ptr& sc) { if (sc != nullptr) { sc->cancelAlarmForSubscriberTriggering(); StatsdStats::getInstance().noteRegisteredPeriodicAlarmChanged(); } })), - mEventQueue(queue) { + mEventQueue(queue), + mStatsCompanionServiceDeathRecipient(AIBinder_DeathRecipient_new( + StatsService::statsCompanionServiceDied)) { mUidMap = UidMap::getInstance(); mPullerManager = new StatsPullerManager(); StatsPuller::SetUidMap(mUidMap); @@ -169,22 +126,20 @@ StatsService::StatsService(const sp& handlerLooper, shared_ptr receiver = mConfigManager->GetConfigReceiver(key); + shared_ptr receiver = mConfigManager->GetConfigReceiver(key); if (receiver == nullptr) { - VLOG("Could not find a broadcast receiver for %s", - key.ToString().c_str()); + VLOG("Could not find a broadcast receiver for %s", key.ToString().c_str()); return false; } else if (receiver->sendDataBroadcast( mProcessor->getLastReportTimeNs(key)).isOk()) { return true; } else { - VLOG("Failed to send a broadcast for receiver %s", - key.ToString().c_str()); + VLOG("Failed to send a broadcast for receiver %s", key.ToString().c_str()); return false; } }, [this](const int& uid, const vector& activeConfigs) { - sp receiver = + shared_ptr receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid); if (receiver == nullptr) { VLOG("Could not find receiver for uid %d", uid); @@ -244,36 +199,6 @@ void StatsService::init_build_type_callback(void* cookie, const char* /*name*/, } } -/** - * Implement our own because the default binder implementation isn't - * properly handling SHELL_COMMAND_TRANSACTION. - */ -status_t StatsService::onTransact(uint32_t code, const Parcel& data, Parcel* reply, - uint32_t flags) { - switch (code) { - case SHELL_COMMAND_TRANSACTION: { - int in = data.readFileDescriptor(); - int out = data.readFileDescriptor(); - int err = data.readFileDescriptor(); - int argc = data.readInt32(); - Vector args; - for (int i = 0; i < argc && data.dataAvail() > 0; i++) { - args.add(String8(data.readString16())); - } - sp shellCallback = IShellCallback::asInterface(data.readStrongBinder()); - sp resultReceiver = - IResultReceiver::asInterface(data.readStrongBinder()); - - err = command(in, out, err, args, resultReceiver); - if (resultReceiver != nullptr) { - resultReceiver->send(err); - } - return NO_ERROR; - } - default: { return BnStatsd::onTransact(code, data, reply, flags); } - } -} - /** * Write data from statsd. * Format for statsdStats: adb shell dumpsys stats --metadata [-v] [--proto] @@ -283,20 +208,21 @@ status_t StatsService::onTransact(uint32_t code, const Parcel& data, Parcel* rep * (bugreports call "adb shell dumpsys stats --dump-priority NORMAL -a --proto") * TODO: Come up with a more robust method of enacting . */ -status_t StatsService::dump(int fd, const Vector& args) { +status_t StatsService::dump(int fd, const char** args, uint32_t numArgs) { if (!checkPermission(kPermissionDump)) { return PERMISSION_DENIED; } - int lastArg = args.size() - 1; + + int lastArg = numArgs - 1; bool asProto = false; - if (lastArg >= 0 && !args[lastArg].compare(String16("--proto"))) { // last argument + if (lastArg >= 0 && string(args[lastArg]) == "--proto") { // last argument asProto = true; lastArg--; } - if (args.size() > 0 && !args[0].compare(String16("--metadata"))) { // first argument + if (numArgs > 0 && string(args[0]) == "--metadata") { // first argument // Request is to dump statsd stats. bool verbose = false; - if (lastArg >= 0 && !args[lastArg].compare(String16("-v"))) { + if (lastArg >= 0 && string(args[lastArg]) == "-v") { verbose = true; lastArg--; } @@ -351,67 +277,72 @@ void StatsService::dumpIncidentSection(int out) { /** * Implementation of the adb shell cmd stats command. */ -status_t StatsService::command(int in, int out, int err, Vector& args, - sp resultReceiver) { - uid_t uid = IPCThreadState::self()->getCallingUid(); - if (uid != AID_ROOT && uid != AID_SHELL) { +status_t StatsService::handleShellCommand(int in, int out, int err, const char** argv, + uint32_t argc) { + uid_t uid = AIBinder_getCallingUid(); + if (uid != AID_ROOT || uid != AID_SHELL) { return PERMISSION_DENIED; } - const int argCount = args.size(); - if (argCount >= 1) { + Vector utf8Args; + utf8Args.setCapacity(argc); + for (uint32_t i = 0; i < argc; i++) { + utf8Args.push(String8(argv[i])); + } + + if (argc >= 1) { // adb shell cmd stats config ... - if (!args[0].compare(String8("config"))) { - return cmd_config(in, out, err, args); + if (!utf8Args[0].compare(String8("config"))) { + return cmd_config(in, out, err, utf8Args); } - if (!args[0].compare(String8("print-uid-map"))) { - return cmd_print_uid_map(out, args); + if (!utf8Args[0].compare(String8("print-uid-map"))) { + return cmd_print_uid_map(out, utf8Args); } - if (!args[0].compare(String8("dump-report"))) { - return cmd_dump_report(out, args); + if (!utf8Args[0].compare(String8("dump-report"))) { + return cmd_dump_report(out, utf8Args); } - if (!args[0].compare(String8("pull-source")) && args.size() > 1) { - return cmd_print_pulled_metrics(out, args); + if (!utf8Args[0].compare(String8("pull-source")) && argc > 1) { + return cmd_print_pulled_metrics(out, utf8Args); } - if (!args[0].compare(String8("send-broadcast"))) { - return cmd_trigger_broadcast(out, args); + if (!utf8Args[0].compare(String8("send-broadcast"))) { + return cmd_trigger_broadcast(out, utf8Args); } - if (!args[0].compare(String8("print-stats"))) { - return cmd_print_stats(out, args); + if (!utf8Args[0].compare(String8("print-stats"))) { + return cmd_print_stats(out, utf8Args); } - if (!args[0].compare(String8("meminfo"))) { + if (!utf8Args[0].compare(String8("meminfo"))) { return cmd_dump_memory_info(out); } - if (!args[0].compare(String8("write-to-disk"))) { + if (!utf8Args[0].compare(String8("write-to-disk"))) { return cmd_write_data_to_disk(out); } - if (!args[0].compare(String8("log-app-breadcrumb"))) { - return cmd_log_app_breadcrumb(out, args); + if (!utf8Args[0].compare(String8("log-app-breadcrumb"))) { + return cmd_log_app_breadcrumb(out, utf8Args); } - if (!args[0].compare(String8("log-binary-push"))) { - return cmd_log_binary_push(out, args); + if (!utf8Args[0].compare(String8("log-binary-push"))) { + return cmd_log_binary_push(out, utf8Args); } - if (!args[0].compare(String8("clear-puller-cache"))) { + if (!utf8Args[0].compare(String8("clear-puller-cache"))) { return cmd_clear_puller_cache(out); } - if (!args[0].compare(String8("print-logs"))) { - return cmd_print_logs(out, args); + if (!utf8Args[0].compare(String8("print-logs"))) { + return cmd_print_logs(out, utf8Args); } - if (!args[0].compare(String8("send-active-configs"))) { - return cmd_trigger_active_config_broadcast(out, args); + if (!utf8Args[0].compare(String8("send-active-configs"))) { + return cmd_trigger_active_config_broadcast(out, utf8Args); } - if (!args[0].compare(String8("data-subscribe"))) { + if (!utf8Args[0].compare(String8("data-subscribe"))) { { std::lock_guard lock(mShellSubscriberMutex); if (mShellSubscriber == nullptr) { @@ -419,14 +350,10 @@ status_t StatsService::command(int in, int out, int err, Vector& args, } } int timeoutSec = -1; - if (argCount >= 2) { - timeoutSec = atoi(args[1].c_str()); + if (argc >= 2) { + timeoutSec = atoi(utf8Args[1].c_str()); } - if (resultReceiver == nullptr) { - ALOGI("Null resultReceiver given, no subscription will be started"); - return UNEXPECTED_NULL; - } - mShellSubscriber->startNewSubscription(in, out, resultReceiver, timeoutSec); + mShellSubscriber->startNewSubscription(in, out, timeoutSec); return NO_ERROR; } } @@ -556,7 +483,7 @@ status_t StatsService::cmd_trigger_broadcast(int out, Vector& args) { const int argCount = args.size(); if (argCount == 2) { // Automatically pick the UID - uid = IPCThreadState::self()->getCallingUid(); + uid = AIBinder_getCallingUid(); name.assign(args[1].c_str(), args[1].size()); good = true; } else if (argCount == 3) { @@ -572,17 +499,15 @@ status_t StatsService::cmd_trigger_broadcast(int out, Vector& args) { return UNKNOWN_ERROR; } ConfigKey key(uid, StrToInt64(name)); - sp receiver = mConfigManager->GetConfigReceiver(key); + shared_ptr receiver = mConfigManager->GetConfigReceiver(key); if (receiver == nullptr) { VLOG("Could not find receiver for %s, %s", args[1].c_str(), args[2].c_str()); return UNKNOWN_ERROR; - } else if (receiver->sendDataBroadcast( - mProcessor->getLastReportTimeNs(key)).isOk()) { + } else if (receiver->sendDataBroadcast(mProcessor->getLastReportTimeNs(key)).isOk()) { VLOG("StatsService::trigger broadcast succeeded to %s, %s", args[1].c_str(), args[2].c_str()); } else { - VLOG("StatsService::trigger broadcast failed to %s, %s", args[1].c_str(), - args[2].c_str()); + VLOG("StatsService::trigger broadcast failed to %s, %s", args[1].c_str(), args[2].c_str()); return UNKNOWN_ERROR; } return NO_ERROR; @@ -594,7 +519,7 @@ status_t StatsService::cmd_trigger_active_config_broadcast(int out, Vector configIds; if (argCount == 1) { // Automatically pick the uid and send a broadcast that has no active configs. - uid = IPCThreadState::self()->getCallingUid(); + uid = AIBinder_getCallingUid(); mProcessor->GetActiveConfigs(uid, configIds); } else { int curArg = 1; @@ -608,7 +533,7 @@ status_t StatsService::cmd_trigger_active_config_broadcast(int out, VectorgetCallingUid(); + uid = AIBinder_getCallingUid(); } if (curArg == argCount || args[curArg] != "--configs") { VLOG("Reached end of args, or specify configs not set. Sending actual active configs,"); @@ -628,7 +553,7 @@ status_t StatsService::cmd_trigger_active_config_broadcast(int out, Vector receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid); + shared_ptr receiver = mConfigManager->GetActiveConfigsChangedReceiver(uid); if (receiver == nullptr) { VLOG("Could not find receiver for uid %d", uid); return UNKNOWN_ERROR; @@ -651,7 +576,7 @@ status_t StatsService::cmd_config(int in, int out, int err, Vector& arg if (argCount == 3) { // Automatically pick the UID - uid = IPCThreadState::self()->getCallingUid(); + uid = AIBinder_getCallingUid(); name.assign(args[2].c_str(), args[2].size()); good = true; } else if (argCount == 4) { @@ -734,7 +659,7 @@ status_t StatsService::cmd_dump_report(int out, const Vector& args) { } if (argCount == 2) { // Automatically pick the UID - uid = IPCThreadState::self()->getCallingUid(); + uid = AIBinder_getCallingUid(); name.assign(args[1].c_str(), args[1].size()); good = true; } else if (argCount == 3) { @@ -826,7 +751,7 @@ status_t StatsService::cmd_log_app_breadcrumb(int out, const Vector& ar const int argCount = args.size(); if (argCount == 3) { // Automatically pick the UID - uid = IPCThreadState::self()->getCallingUid(); + uid = AIBinder_getCallingUid(); label = atoi(args[1].c_str()); state = atoi(args[2].c_str()); good = true; @@ -903,9 +828,8 @@ status_t StatsService::cmd_dump_memory_info(int out) { } status_t StatsService::cmd_clear_puller_cache(int out) { - IPCThreadState* ipc = IPCThreadState::self(); VLOG("StatsService::cmd_clear_puller_cache with Pid %i, Uid %i", - ipc->getCallingPid(), ipc->getCallingUid()); + AIBinder_getCallingPid(), AIBinder_getCallingUid()); if (checkPermission(kPermissionDump)) { int cleared = mPullerManager->ForceClearPullerCache(); dprintf(out, "Puller removed %d cached data!\n", cleared); @@ -916,9 +840,8 @@ status_t StatsService::cmd_clear_puller_cache(int out) { } status_t StatsService::cmd_print_logs(int out, const Vector& args) { - IPCThreadState* ipc = IPCThreadState::self(); - VLOG("StatsService::cmd_print_logs with Pid %i, Uid %i", ipc->getCallingPid(), - ipc->getCallingUid()); + VLOG("StatsService::cmd_print_logs with Pid %i, Uid %i", AIBinder_getCallingPid(), + AIBinder_getCallingUid()); if (checkPermission(kPermissionDump)) { bool enabled = true; if (args.size() >= 2) { @@ -950,24 +873,24 @@ bool StatsService::getUidFromString(const char* s, int32_t& uid) { } uid = goodUid; - int32_t callingUid = IPCThreadState::self()->getCallingUid(); + int32_t callingUid = AIBinder_getCallingUid(); return mEngBuild // UserDebug/EngBuild are allowed to impersonate uids. || (callingUid == goodUid) // Anyone can 'impersonate' themselves. || (callingUid == AID_ROOT && goodUid == AID_SHELL); // ROOT can impersonate SHELL. } -Status StatsService::informAllUidData(const ParcelFileDescriptor& fd) { +Status StatsService::informAllUidData(const ScopedFileDescriptor& fd) { ENFORCE_UID(AID_SYSTEM); // Read stream into buffer. string buffer; if (!android::base::ReadFdToString(fd.get(), &buffer)) { - return exception(Status::EX_ILLEGAL_ARGUMENT, "Failed to read all data from the pipe."); + return exception(EX_ILLEGAL_ARGUMENT, "Failed to read all data from the pipe."); } // Parse buffer. UidData uidData; if (!uidData.ParseFromString(buffer)) { - return exception(Status::EX_ILLEGAL_ARGUMENT, "Error parsing proto stream for UidData."); + return exception(EX_ILLEGAL_ARGUMENT, "Error parsing proto stream for UidData."); } vector versionStrings; @@ -1002,20 +925,28 @@ Status StatsService::informAllUidData(const ParcelFileDescriptor& fd) { return Status::ok(); } -Status StatsService::informOnePackage(const String16& app, int32_t uid, int64_t version, - const String16& version_string, const String16& installer) { +Status StatsService::informOnePackage(const string& app, int32_t uid, int64_t version, + const string& versionString, const string& installer) { ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::informOnePackage was called"); - mUidMap->updateApp(getElapsedRealtimeNs(), app, uid, version, version_string, installer); + // TODO(b/149254662): This is gross. We should consider changing statsd + // internals to use std::string. + String16 utf16App = String16(app.c_str()); + String16 utf16VersionString = String16(versionString.c_str()); + String16 utf16Installer = String16(installer.c_str()); + + mUidMap->updateApp(getElapsedRealtimeNs(), utf16App, uid, version, utf16VersionString, + utf16Installer); return Status::ok(); } -Status StatsService::informOnePackageRemoved(const String16& app, int32_t uid) { +Status StatsService::informOnePackageRemoved(const string& app, int32_t uid) { ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::informOnePackageRemoved was called"); - mUidMap->removeApp(getElapsedRealtimeNs(), app, uid); + String16 utf16App = String16(app.c_str()); + mUidMap->removeApp(getElapsedRealtimeNs(), utf16App, uid); mConfigManager->RemoveConfigs(uid); return Status::ok(); } @@ -1079,7 +1010,7 @@ Status StatsService::informDeviceShutdown() { } void StatsService::sayHiToStatsCompanion() { - sp statsCompanion = getStatsCompanionService(); + shared_ptr statsCompanion = getStatsCompanionService(); if (statsCompanion != nullptr) { VLOG("Telling statsCompanion that statsd is ready"); statsCompanion->statsdReady(); @@ -1092,14 +1023,14 @@ Status StatsService::statsCompanionReady() { ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::statsCompanionReady was called"); - sp statsCompanion = getStatsCompanionService(); + shared_ptr statsCompanion = getStatsCompanionService(); if (statsCompanion == nullptr) { - return Status::fromExceptionCode( - Status::EX_NULL_POINTER, - "statscompanion unavailable despite it contacting statsd!"); + return exception(EX_NULL_POINTER, + "StatsCompanion unavailable despite it contacting statsd."); } VLOG("StatsService::statsCompanionReady linking to statsCompanion."); - IInterface::asBinder(statsCompanion)->linkToDeath(this); + AIBinder_linkToDeath(statsCompanion->asBinder().get(), + mStatsCompanionServiceDeathRecipient.get(), this); mPullerManager->SetStatsCompanionService(statsCompanion); mAnomalyAlarmMonitor->setStatsCompanionService(statsCompanion); mPeriodicAlarmMonitor->setStatsCompanionService(statsCompanion); @@ -1127,39 +1058,47 @@ void StatsService::OnLogEvent(LogEvent* event) { } } -Status StatsService::getData(int64_t key, const int32_t callingUid, vector* output) { +Status StatsService::getData(int64_t key, const int32_t callingUid, vector* output) { ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::getData with Uid %i", callingUid); ConfigKey configKey(callingUid, key); + // TODO(b/149254662): Since libbinder_ndk uses int8_t instead of uint8_t, + // there are inconsistencies with internal statsd logic. Instead of + // modifying lots of files, we create a temporary output array of int8_t and + // copy its data into output. This is a bad hack, but hopefully + // libbinder_ndk will transition to using uint8_t soon: progress is tracked + // in b/144957764. Same applies to StatsService::getMetadata. + vector unsignedOutput; // The dump latency does not matter here since we do not include the current bucket, we do not // need to pull any new data anyhow. mProcessor->onDumpReport(configKey, getElapsedRealtimeNs(), false /* include_current_bucket*/, - true /* erase_data */, GET_DATA_CALLED, FAST, output); + true /* erase_data */, GET_DATA_CALLED, FAST, &unsignedOutput); + *output = vector(unsignedOutput.begin(), unsignedOutput.end()); return Status::ok(); } -Status StatsService::getMetadata(vector* output) { +Status StatsService::getMetadata(vector* output) { ENFORCE_UID(AID_SYSTEM); - StatsdStats::getInstance().dumpStats(output, false); // Don't reset the counters. + vector unsignedOutput; + StatsdStats::getInstance().dumpStats(&unsignedOutput, false); // Don't reset the counters. + *output = vector(unsignedOutput.begin(), unsignedOutput.end()); return Status::ok(); } -Status StatsService::addConfiguration(int64_t key, const vector & config, +Status StatsService::addConfiguration(int64_t key, const vector & config, const int32_t callingUid) { ENFORCE_UID(AID_SYSTEM); if (addConfigurationChecked(callingUid, key, config)) { return Status::ok(); } else { - ALOGE("Could not parse malformatted StatsdConfig"); - return Status::fromExceptionCode(binder::Status::EX_ILLEGAL_ARGUMENT, - "config does not correspond to a StatsdConfig proto"); + return exception(EX_ILLEGAL_ARGUMENT, "Could not parse malformatted StatsdConfig."); } } -bool StatsService::addConfigurationChecked(int uid, int64_t key, const vector& config) { +bool StatsService::addConfigurationChecked(int uid, int64_t key, const vector& config) { ConfigKey configKey(uid, key); StatsdConfig cfg; if (config.size() > 0) { // If the config is empty, skip parsing. @@ -1180,7 +1119,7 @@ Status StatsService::removeDataFetchOperation(int64_t key, } Status StatsService::setDataFetchOperation(int64_t key, - const sp& pir, + const shared_ptr& pir, const int32_t callingUid) { ENFORCE_UID(AID_SYSTEM); @@ -1194,7 +1133,7 @@ Status StatsService::setDataFetchOperation(int64_t key, return Status::ok(); } -Status StatsService::setActiveConfigsChangedOperation(const sp& pir, +Status StatsService::setActiveConfigsChangedOperation(const shared_ptr& pir, const int32_t callingUid, vector* output) { ENFORCE_UID(AID_SYSTEM); @@ -1225,7 +1164,7 @@ Status StatsService::removeConfiguration(int64_t key, const int32_t callingUid) Status StatsService::setBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const sp& pir, + const shared_ptr& pir, const int32_t callingUid) { ENFORCE_UID(AID_SYSTEM); @@ -1252,14 +1191,14 @@ Status StatsService::sendAppBreadcrumbAtom(int32_t label, int32_t state) { // Permission check not necessary as it's meant for applications to write to // statsd. android::util::stats_write(util::APP_BREADCRUMB_REPORTED, - (int32_t) IPCThreadState::self()->getCallingUid(), label, + (int32_t) AIBinder_getCallingUid(), label, state); return Status::ok(); } Status StatsService::registerPullAtomCallback(int32_t uid, int32_t atomTag, int64_t coolDownNs, int64_t timeoutNs, const std::vector& additiveFields, - const sp& pullerCallback) { + const shared_ptr& pullerCallback) { ENFORCE_UID(AID_SYSTEM); VLOG("StatsService::registerPullAtomCallback called."); @@ -1270,10 +1209,10 @@ Status StatsService::registerPullAtomCallback(int32_t uid, int32_t atomTag, int6 Status StatsService::registerNativePullAtomCallback(int32_t atomTag, int64_t coolDownNs, int64_t timeoutNs, const std::vector& additiveFields, - const sp& pullerCallback) { + const shared_ptr& pullerCallback) { VLOG("StatsService::registerNativePullAtomCallback called."); - int32_t uid = IPCThreadState::self()->getCallingUid(); + int32_t uid = AIBinder_getCallingUid(); mPullerManager->RegisterPullAtomCallback(uid, atomTag, coolDownNs, timeoutNs, additiveFields, pullerCallback); return Status::ok(); @@ -1288,7 +1227,7 @@ Status StatsService::unregisterPullAtomCallback(int32_t uid, int32_t atomTag) { Status StatsService::unregisterNativePullAtomCallback(int32_t atomTag) { VLOG("StatsService::unregisterNativePullAtomCallback called."); - int32_t uid = IPCThreadState::self()->getCallingUid(); + int32_t uid = AIBinder_getCallingUid(); mPullerManager->UnregisterPullAtomCallback(uid, atomTag); return Status::ok(); } @@ -1314,7 +1253,13 @@ Status StatsService::getRegisteredExperimentIds(std::vector* experiment return Status::ok(); } -void StatsService::binderDied(const wp & who) { + +void StatsService::statsCompanionServiceDied(void* cookie) { + auto thiz = static_cast(cookie); + thiz->statsCompanionServiceDiedImpl(); +} + +void StatsService::statsCompanionServiceDiedImpl() { ALOGW("statscompanion service died"); StatsdStats::getInstance().noteSystemServerRestart(getWallClockSec()); if (mProcessor != nullptr) { diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h index 0527d435fdf49..e6723c83e02a4 100644 --- a/cmds/statsd/src/StatsService.h +++ b/cmds/statsd/src/StatsService.h @@ -27,27 +27,30 @@ #include "shell/ShellSubscriber.h" #include "statscompanion_util.h" -#include -#include -#include -#include -#include -#include +#include +#include +#include #include #include using namespace android; -using namespace android::binder; using namespace android::os; using namespace std; +using Status = ::ndk::ScopedAStatus; +using aidl::android::os::BnStatsd; +using aidl::android::os::IPendingIntentRef; +using aidl::android::os::IPullAtomCallback; +using ::ndk::ScopedAIBinder_DeathRecipient; +using ::ndk::ScopedFileDescriptor; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { -class StatsService : public BnStatsd, - public IBinder::DeathRecipient { +class StatsService : public BnStatsd { public: StatsService(const sp& handlerLooper, std::shared_ptr queue); virtual ~StatsService(); @@ -55,10 +58,9 @@ public: /** The anomaly alarm registered with AlarmManager won't be updated by less than this. */ const uint32_t MIN_DIFF_TO_UPDATE_REGISTERED_ALARM_SECS = 5; - virtual status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags); - virtual status_t dump(int fd, const Vector& args); - virtual status_t command(int inFd, int outFd, int err, Vector& args, - sp resultReceiver); + virtual status_t dump(int fd, const char** args, uint32_t numArgs) override; + virtual status_t handleShellCommand(int in, int out, int err, const char** argv, + uint32_t argc) override; virtual Status systemRunning(); virtual Status statsCompanionReady(); @@ -66,10 +68,10 @@ public: virtual Status informPollAlarmFired(); virtual Status informAlarmForSubscriberTriggeringFired(); - virtual Status informAllUidData(const ParcelFileDescriptor& fd); - virtual Status informOnePackage(const String16& app, int32_t uid, int64_t version, - const String16& version_string, const String16& installer); - virtual Status informOnePackageRemoved(const String16& app, int32_t uid); + virtual Status informAllUidData(const ScopedFileDescriptor& fd); + virtual Status informOnePackage(const string& app, int32_t uid, int64_t version, + const string& versionString, const string& installer); + virtual Status informOnePackageRemoved(const string& app, int32_t uid); virtual Status informDeviceShutdown(); /** @@ -92,13 +94,13 @@ public: */ virtual Status getData(int64_t key, const int32_t callingUid, - vector* output) override; + vector* output) override; /** * Binder call for clients to get metadata across all configs in statsd. */ - virtual Status getMetadata(vector* output) override; + virtual Status getMetadata(vector* output) override; /** @@ -106,14 +108,14 @@ public: * should requestData for this configuration. */ virtual Status addConfiguration(int64_t key, - const vector& config, + const vector& config, const int32_t callingUid) override; /** * Binder call to let clients register the data fetch operation for a configuration. */ virtual Status setDataFetchOperation(int64_t key, - const sp& pir, + const shared_ptr& pir, const int32_t callingUid) override; /** @@ -125,7 +127,7 @@ public: /** * Binder call to let clients register the active configs changed operation. */ - virtual Status setActiveConfigsChangedOperation(const sp& pir, + virtual Status setActiveConfigsChangedOperation(const shared_ptr& pir, const int32_t callingUid, vector* output) override; @@ -144,7 +146,7 @@ public: */ virtual Status setBroadcastSubscriber(int64_t configId, int64_t subscriberId, - const sp& pir, + const shared_ptr& pir, const int32_t callingUid) override; /** @@ -167,14 +169,14 @@ public: */ virtual Status registerPullAtomCallback(int32_t uid, int32_t atomTag, int64_t coolDownNs, int64_t timeoutNs, const std::vector& additiveFields, - const sp& pullerCallback) override; + const shared_ptr& pullerCallback) override; /** * Binder call to register a callback function for a pulled atom. */ virtual Status registerNativePullAtomCallback(int32_t atomTag, int64_t coolDownNs, int64_t timeoutNs, const std::vector& additiveFields, - const sp& pullerCallback) override; + const shared_ptr& pullerCallback) override; /** * Binder call to unregister any existing callback for the given uid and atom. @@ -191,9 +193,6 @@ public: */ virtual Status getRegisteredExperimentIds(std::vector* expIdsOut); - /** IBinder::DeathRecipient */ - virtual void binderDied(const wp& who) override; - private: /** * Load system properties at init. @@ -317,13 +316,24 @@ private: /** * Adds a configuration after checking permissions and obtaining UID from binder call. */ - bool addConfigurationChecked(int uid, int64_t key, const vector& config); + bool addConfigurationChecked(int uid, int64_t key, const vector& config); /** * Update a configuration. */ void set_config(int uid, const string& name, const StatsdConfig& config); + /** + * Death recipient callback that is called when StatsCompanionService dies. + * The cookie is a pointer to a StatsService object. + */ + static void statsCompanionServiceDied(void* cookie); + + /** + * Implementation of statsCompanionServiceDied. + */ + void statsCompanionServiceDiedImpl(); + /** * Tracks the uid <--> package name mapping. */ @@ -367,6 +377,8 @@ private: mutable mutex mShellSubscriberMutex; std::shared_ptr mEventQueue; + ScopedAIBinder_DeathRecipient mStatsCompanionServiceDeathRecipient; + FRIEND_TEST(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart); FRIEND_TEST(StatsServiceTest, TestAddConfig_simple); FRIEND_TEST(StatsServiceTest, TestAddConfig_empty); diff --git a/cmds/statsd/src/anomaly/AlarmMonitor.cpp b/cmds/statsd/src/anomaly/AlarmMonitor.cpp index bc36dadacddb0..02291181b81b6 100644 --- a/cmds/statsd/src/anomaly/AlarmMonitor.cpp +++ b/cmds/statsd/src/anomaly/AlarmMonitor.cpp @@ -26,17 +26,21 @@ namespace statsd { AlarmMonitor::AlarmMonitor( uint32_t minDiffToUpdateRegisteredAlarmTimeSec, - const std::function&, int64_t)>& updateAlarm, - const std::function&)>& cancelAlarm) - : mRegisteredAlarmTimeSec(0), mMinUpdateTimeSec(minDiffToUpdateRegisteredAlarmTimeSec), + const std::function&, int64_t)>& updateAlarm, + const std::function&)>& cancelAlarm) + : mRegisteredAlarmTimeSec(0), + mMinUpdateTimeSec(minDiffToUpdateRegisteredAlarmTimeSec), mUpdateAlarm(updateAlarm), mCancelAlarm(cancelAlarm) {} AlarmMonitor::~AlarmMonitor() {} -void AlarmMonitor::setStatsCompanionService(sp statsCompanionService) { +void AlarmMonitor::setStatsCompanionService( + shared_ptr statsCompanionService) { std::lock_guard lock(mLock); - sp tmpForLock = mStatsCompanionService; + // TODO(b/149254662): determine if tmpForLock is needed now that we have moved + // from sp to shared_ptr + shared_ptr tmpForLock = mStatsCompanionService; mStatsCompanionService = statsCompanionService; if (statsCompanionService == nullptr) { VLOG("Erasing link to statsCompanionService"); diff --git a/cmds/statsd/src/anomaly/AlarmMonitor.h b/cmds/statsd/src/anomaly/AlarmMonitor.h index 219695ef5e43f..5c34e381ba0b6 100644 --- a/cmds/statsd/src/anomaly/AlarmMonitor.h +++ b/cmds/statsd/src/anomaly/AlarmMonitor.h @@ -18,7 +18,7 @@ #include "anomaly/indexed_priority_queue.h" -#include +#include #include #include @@ -26,7 +26,9 @@ using namespace android; -using android::os::IStatsCompanionService; +using aidl::android::os::IStatsCompanionService; +using std::function; +using std::shared_ptr; using std::unordered_set; namespace android { @@ -64,8 +66,9 @@ public: * alarm. */ AlarmMonitor(uint32_t minDiffToUpdateRegisteredAlarmTimeSec, - const std::function&, int64_t)>& updateAlarm, - const std::function&)>& cancelAlarm); + const function&, int64_t)>& + updateAlarm, + const function&)>& cancelAlarm); ~AlarmMonitor(); /** @@ -74,7 +77,7 @@ public: * If nullptr, AnomalyMonitor will continue to add/remove alarms, but won't * update IStatsCompanionService (until such time as it is set non-null). */ - void setStatsCompanionService(sp statsCompanionService); + void setStatsCompanionService(shared_ptr statsCompanionService); /** * Adds the given alarm (reference) to the queue. @@ -123,7 +126,7 @@ private: /** * Binder interface for communicating with StatsCompanionService. */ - sp mStatsCompanionService = nullptr; + shared_ptr mStatsCompanionService = nullptr; /** * Amount by which the soonest projected alarm must differ from @@ -147,10 +150,10 @@ private: int64_t secToMs(uint32_t timeSec); // Callback function to update the alarm via StatsCompanionService. - std::function, int64_t)> mUpdateAlarm; + std::function, int64_t)> mUpdateAlarm; // Callback function to cancel the alarm via StatsCompanionService. - std::function)> mCancelAlarm; + std::function)> mCancelAlarm; }; diff --git a/cmds/statsd/src/anomaly/AlarmTracker.h b/cmds/statsd/src/anomaly/AlarmTracker.h index 2fb3e3b3e26a1..2da4a18682ae8 100644 --- a/cmds/statsd/src/anomaly/AlarmTracker.h +++ b/cmds/statsd/src/anomaly/AlarmTracker.h @@ -22,12 +22,9 @@ #include "config/ConfigKey.h" #include "frameworks/base/cmds/statsd/src/statsd_config.pb.h" // Alarm -#include #include #include -using android::os::IStatsCompanionService; - namespace android { namespace os { namespace statsd { diff --git a/cmds/statsd/src/config/ConfigManager.cpp b/cmds/statsd/src/config/ConfigManager.cpp index 986955bc5f39a..9bdb5881cc944 100644 --- a/cmds/statsd/src/config/ConfigManager.cpp +++ b/cmds/statsd/src/config/ConfigManager.cpp @@ -42,42 +42,67 @@ using std::vector; using android::base::StringPrintf; using std::unique_ptr; -class ConfigReceiverDeathRecipient : public android::IBinder::DeathRecipient { - public: - ConfigReceiverDeathRecipient(sp configManager, const ConfigKey& configKey): - mConfigManager(configManager), - mConfigKey(configKey) {} - ~ConfigReceiverDeathRecipient() override = default; - private: - sp mConfigManager; - ConfigKey mConfigKey; +struct ConfigReceiverDeathCookie { + ConfigReceiverDeathCookie(sp configManager, const ConfigKey& configKey, + const shared_ptr& pir): + mConfigManager(configManager), + mConfigKey(configKey), + mPir(pir) {} - void binderDied(const android::wp& who) override { - if (IInterface::asBinder(mConfigManager->GetConfigReceiver(mConfigKey)) == who.promote()) { - mConfigManager->RemoveConfigReceiver(mConfigKey); - } - } + sp mConfigManager; + ConfigKey mConfigKey; + shared_ptr mPir; }; -class ActiveConfigChangedReceiverDeathRecipient : public android::IBinder::DeathRecipient { - public: - ActiveConfigChangedReceiverDeathRecipient(sp configManager, const int uid): - mConfigManager(configManager), - mUid(uid) {} - ~ActiveConfigChangedReceiverDeathRecipient() override = default; - private: - sp mConfigManager; - int mUid; +static void configReceiverDied(void* cookie) { + auto cookie_ = static_cast(cookie); + sp configManager = cookie_->mConfigManager; + ConfigKey configKey = cookie_->mConfigKey; + shared_ptr pir = cookie_->mPir; - void binderDied(const android::wp& who) override { - if (IInterface::asBinder(mConfigManager->GetActiveConfigsChangedReceiver(mUid)) - == who.promote()) { - mConfigManager->RemoveActiveConfigsChangedReceiver(mUid); - } + // TODO(b/149254662): Fix threading. This currently fails if a new + // pir gets set between the get and the remove. + if (configManager->GetConfigReceiver(configKey) == pir) { + configManager->RemoveConfigReceiver(configKey); } + // The death recipient corresponding to this specific pir can never + // be triggered again, so free up resources. + // TODO(b/149254662): Investigate other options to manage the memory. + delete cookie_; +} + +struct ActiveConfigChangedReceiverDeathCookie { + ActiveConfigChangedReceiverDeathCookie(sp configManager, const int uid, + const shared_ptr& pir): + mConfigManager(configManager), + mUid(uid), + mPir(pir) {} + + sp mConfigManager; + int mUid; + shared_ptr mPir; }; -ConfigManager::ConfigManager() { +static void activeConfigChangedReceiverDied(void* cookie) { + auto cookie_ = static_cast(cookie); + sp configManager = cookie_->mConfigManager; + int uid = cookie_->mUid; + shared_ptr pir = cookie_->mPir; + + // TODO(b/149254662): Fix threading. This currently fails if a new + // pir gets set between the get and the remove. + if (configManager->GetActiveConfigsChangedReceiver(uid) == pir) { + configManager->RemoveActiveConfigsChangedReceiver(uid); + } + // The death recipient corresponding to this specific pir can never + // be triggered again, so free up resources. + delete cookie_; +} + +ConfigManager::ConfigManager(): + mConfigReceiverDeathRecipient(AIBinder_DeathRecipient_new(configReceiverDied)), + mActiveConfigChangedReceiverDeathRecipient( + AIBinder_DeathRecipient_new(activeConfigChangedReceiverDied)) { } ConfigManager::~ConfigManager() { @@ -150,10 +175,11 @@ void ConfigManager::UpdateConfig(const ConfigKey& key, const StatsdConfig& confi } void ConfigManager::SetConfigReceiver(const ConfigKey& key, - const sp& pir) { + const shared_ptr& pir) { lock_guard lock(mMutex); mConfigReceivers[key] = pir; - IInterface::asBinder(pir)->linkToDeath(new ConfigReceiverDeathRecipient(this, key)); + AIBinder_linkToDeath(pir->asBinder().get(), mConfigReceiverDeathRecipient.get(), + new ConfigReceiverDeathCookie(this, key, pir)); } void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) { @@ -162,11 +188,11 @@ void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) { } void ConfigManager::SetActiveConfigsChangedReceiver(const int uid, - const sp& pir) { + const shared_ptr& pir) { lock_guard lock(mMutex); mActiveConfigsChangedReceivers[uid] = pir; - IInterface::asBinder(pir)->linkToDeath( - new ActiveConfigChangedReceiverDeathRecipient(this, uid)); + AIBinder_linkToDeath(pir->asBinder().get(), mActiveConfigChangedReceiverDeathRecipient.get(), + new ActiveConfigChangedReceiverDeathCookie(this, uid, pir)); } void ConfigManager::RemoveActiveConfigsChangedReceiver(const int uid) { @@ -279,7 +305,7 @@ vector ConfigManager::GetAllConfigKeys() const { return ret; } -const sp ConfigManager::GetConfigReceiver(const ConfigKey& key) const { +const shared_ptr ConfigManager::GetConfigReceiver(const ConfigKey& key) const { lock_guard lock(mMutex); auto it = mConfigReceivers.find(key); @@ -290,7 +316,8 @@ const sp ConfigManager::GetConfigReceiver(const ConfigKey& ke } } -const sp ConfigManager::GetActiveConfigsChangedReceiver(const int uid) const { +const shared_ptr ConfigManager::GetActiveConfigsChangedReceiver(const int uid) + const { lock_guard lock(mMutex); auto it = mActiveConfigsChangedReceivers.find(uid); diff --git a/cmds/statsd/src/config/ConfigManager.h b/cmds/statsd/src/config/ConfigManager.h index 2095173e8959f..824e58849b789 100644 --- a/cmds/statsd/src/config/ConfigManager.h +++ b/cmds/statsd/src/config/ConfigManager.h @@ -19,12 +19,15 @@ #include "config/ConfigKey.h" #include "config/ConfigListener.h" -#include +#include #include #include #include +using aidl::android::os::IPendingIntentRef; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { @@ -62,12 +65,12 @@ public: /** * Sets the broadcast receiver for a configuration key. */ - void SetConfigReceiver(const ConfigKey& key, const sp& pir); + void SetConfigReceiver(const ConfigKey& key, const shared_ptr& pir); /** * Returns the package name and class name representing the broadcast receiver for this config. */ - const sp GetConfigReceiver(const ConfigKey& key) const; + const shared_ptr GetConfigReceiver(const ConfigKey& key) const; /** * Returns all config keys registered. @@ -83,13 +86,13 @@ public: * Sets the broadcast receiver that is notified whenever the list of active configs * changes for this uid. */ - void SetActiveConfigsChangedReceiver(const int uid, const sp& pir); + void SetActiveConfigsChangedReceiver(const int uid, const shared_ptr& pir); /** * Returns the broadcast receiver for active configs changed for this uid. */ - const sp GetActiveConfigsChangedReceiver(const int uid) const; + const shared_ptr GetActiveConfigsChangedReceiver(const int uid) const; /** * Erase any active configs changed broadcast receiver associated with this uid. @@ -141,18 +144,23 @@ private: /** * Each config key can be subscribed by up to one receiver, specified as IPendingIntentRef. */ - std::map> mConfigReceivers; + std::map> mConfigReceivers; /** * Each uid can be subscribed by up to one receiver to notify that the list of active configs * for this uid has changed. The receiver is specified as IPendingIntentRef. */ - std::map> mActiveConfigsChangedReceivers; + std::map> mActiveConfigsChangedReceivers; /** * The ConfigListeners that will be told about changes. */ std::vector> mListeners; + + // Death recipients that are triggered when the host process holding an + // IPendingIntentRef dies. + ::ndk::ScopedAIBinder_DeathRecipient mConfigReceiverDeathRecipient; + ::ndk::ScopedAIBinder_DeathRecipient mActiveConfigChangedReceiverDeathRecipient; }; } // namespace statsd diff --git a/cmds/statsd/src/external/PullResultReceiver.cpp b/cmds/statsd/src/external/PullResultReceiver.cpp index 6b6fe7d9617f6..8aa4792dc1799 100644 --- a/cmds/statsd/src/external/PullResultReceiver.cpp +++ b/cmds/statsd/src/external/PullResultReceiver.cpp @@ -16,17 +16,12 @@ #include "PullResultReceiver.h" -using namespace android::binder; -using namespace android::util; -using namespace std; - namespace android { namespace os { namespace statsd { PullResultReceiver::PullResultReceiver( - std::function&)> - pullFinishCb) + std::function&)> pullFinishCb) : pullFinishCallback(std::move(pullFinishCb)) { } diff --git a/cmds/statsd/src/external/PullResultReceiver.h b/cmds/statsd/src/external/PullResultReceiver.h index 17d06e4ff4db5..ceaae801b2d5d 100644 --- a/cmds/statsd/src/external/PullResultReceiver.h +++ b/cmds/statsd/src/external/PullResultReceiver.h @@ -14,29 +14,33 @@ * limitations under the License. */ -#include +#include +#include using namespace std; +using Status = ::ndk::ScopedAStatus; +using aidl::android::os::BnPullAtomResultReceiver; +using aidl::android::util::StatsEventParcel; + namespace android { namespace os { namespace statsd { class PullResultReceiver : public BnPullAtomResultReceiver { public: - PullResultReceiver(function&)> + PullResultReceiver(function&)> pullFinishCallback); ~PullResultReceiver(); /** * Binder call for finishing a pull. */ - binder::Status pullFinished(int32_t atomTag, bool success, - const vector& output) override; + Status pullFinished(int32_t atomTag, bool success, + const vector& output) override; private: - function&)> - pullFinishCallback; + function&)> pullFinishCallback; }; } // namespace statsd diff --git a/cmds/statsd/src/external/StatsCallbackPuller.cpp b/cmds/statsd/src/external/StatsCallbackPuller.cpp index 1a11f0e373112..b6c8e3435d00b 100644 --- a/cmds/statsd/src/external/StatsCallbackPuller.cpp +++ b/cmds/statsd/src/external/StatsCallbackPuller.cpp @@ -18,24 +18,24 @@ #include "Log.h" #include "StatsCallbackPuller.h" - -#include -#include - #include "PullResultReceiver.h" #include "StatsPullerManager.h" #include "logd/LogEvent.h" #include "stats_log_util.h" -using namespace android::binder; -using namespace android::util; +#include + using namespace std; +using Status = ::ndk::ScopedAStatus; +using aidl::android::util::StatsEventParcel; +using ::ndk::SharedRefBase; + namespace android { namespace os { namespace statsd { -StatsCallbackPuller::StatsCallbackPuller(int tagId, const sp& callback, +StatsCallbackPuller::StatsCallbackPuller(int tagId, const shared_ptr& callback, const int64_t coolDownNs, int64_t timeoutNs, const vector additiveFields) : StatsPuller(tagId, coolDownNs, timeoutNs, additiveFields), mCallback(callback) { @@ -57,7 +57,7 @@ bool StatsCallbackPuller::PullInternal(vector>* data) { shared_ptr>> sharedData = make_shared>>(); - sp resultReceiver = new PullResultReceiver( + shared_ptr resultReceiver = SharedRefBase::make( [cv_mutex, cv, pullFinish, pullSuccess, sharedData]( int32_t atomTag, bool success, const vector& output) { // This is the result of the pull, executing in a statsd binder thread. @@ -66,9 +66,11 @@ bool StatsCallbackPuller::PullInternal(vector>* data) { { lock_guard lk(*cv_mutex); for (const StatsEventParcel& parcel: output) { + uint8_t* buf = reinterpret_cast( + const_cast(parcel.buffer.data())); shared_ptr event = make_shared( - const_cast(parcel.buffer.data()), parcel.buffer.size(), - /*uid=*/-1, /*pid=*/-1, /*useNewSchema=*/true); + buf, parcel.buffer.size(), /*uid=*/-1, /*pid=*/-1, + /*useNewSchema=*/true); sharedData->push_back(event); } *pullSuccess = success; diff --git a/cmds/statsd/src/external/StatsCallbackPuller.h b/cmds/statsd/src/external/StatsCallbackPuller.h index fe6af19e3901b..e82e8bb532be2 100644 --- a/cmds/statsd/src/external/StatsCallbackPuller.h +++ b/cmds/statsd/src/external/StatsCallbackPuller.h @@ -16,23 +16,25 @@ #pragma once -#include - +#include #include "StatsPuller.h" +using aidl::android::os::IPullAtomCallback; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { class StatsCallbackPuller : public StatsPuller { public: - explicit StatsCallbackPuller(int tagId, const sp& callback, + explicit StatsCallbackPuller(int tagId, const shared_ptr& callback, const int64_t coolDownNs, const int64_t timeoutNs, const std::vector additiveFields); private: - bool PullInternal(vector >* data) override; - const sp mCallback; + bool PullInternal(vector>* data) override; + const shared_ptr mCallback; FRIEND_TEST(StatsCallbackPullerTest, PullFail); FRIEND_TEST(StatsCallbackPullerTest, PullSuccess); diff --git a/cmds/statsd/src/external/StatsPuller.h b/cmds/statsd/src/external/StatsPuller.h index 9c2169565094f..fee571c52a5f4 100644 --- a/cmds/statsd/src/external/StatsPuller.h +++ b/cmds/statsd/src/external/StatsPuller.h @@ -16,7 +16,7 @@ #pragma once -#include +#include #include #include #include @@ -26,6 +26,9 @@ #include "logd/LogEvent.h" #include "puller_util.h" +using aidl::android::os::IStatsCompanionService; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { @@ -58,7 +61,8 @@ public: static void SetUidMap(const sp& uidMap); - virtual void SetStatsCompanionService(sp statsCompanionService){}; + virtual void SetStatsCompanionService( + shared_ptr statsCompanionService) {}; protected: const int mTagId; diff --git a/cmds/statsd/src/external/StatsPullerManager.cpp b/cmds/statsd/src/external/StatsPullerManager.cpp index 3ceff75294400..1c38542b90177 100644 --- a/cmds/statsd/src/external/StatsPullerManager.cpp +++ b/cmds/statsd/src/external/StatsPullerManager.cpp @@ -19,8 +19,6 @@ #include "StatsPullerManager.h" -#include -#include #include #include #include @@ -87,7 +85,10 @@ void StatsPullerManager::updateAlarmLocked() { return; } - sp statsCompanionServiceCopy = mStatsCompanionService; + // TODO(b/149254662): Why are we creating a copy here? This is different + // from the other places where we create a copy because we don't reassign + // mStatsCompanionService so a destructor can't implicitly be called... + shared_ptr statsCompanionServiceCopy = mStatsCompanionService; if (statsCompanionServiceCopy != nullptr) { statsCompanionServiceCopy->setPullingAlarm(mNextPullTimeNs / 1000000); } else { @@ -97,9 +98,11 @@ void StatsPullerManager::updateAlarmLocked() { } void StatsPullerManager::SetStatsCompanionService( - sp statsCompanionService) { + shared_ptr statsCompanionService) { + // TODO(b/149254662): Why are we using AutoMutex instead of lock_guard? + // Additionally, do we need the temporary shared_ptr to prevent deadlocks? AutoMutex _l(mLock); - sp tmpForLock = mStatsCompanionService; + shared_ptr tmpForLock = mStatsCompanionService; mStatsCompanionService = statsCompanionService; for (const auto& pulledAtom : kAllPullAtomInfo) { pulledAtom.second->SetStatsCompanionService(statsCompanionService); @@ -250,10 +253,11 @@ int StatsPullerManager::ClearPullerCacheIfNecessary(int64_t timestampNs) { void StatsPullerManager::RegisterPullAtomCallback(const int uid, const int32_t atomTag, const int64_t coolDownNs, const int64_t timeoutNs, const vector& additiveFields, - const sp& callback) { + const shared_ptr& callback) { AutoMutex _l(mLock); VLOG("RegisterPullerCallback: adding puller for tag %d", atomTag); - // TODO: linkToDeath with the callback so that we can remove it and delete the puller. + // TODO(b/146439412): linkToDeath with the callback so that we can remove it + // and delete the puller. StatsdStats::getInstance().notePullerCallbackRegistrationChanged(atomTag, /*registered=*/true); kAllPullAtomInfo[{.atomTag = atomTag}] = new StatsCallbackPuller(atomTag, callback, coolDownNs, timeoutNs, additiveFields); diff --git a/cmds/statsd/src/external/StatsPullerManager.h b/cmds/statsd/src/external/StatsPullerManager.h index aef16dcf2147b..e067766234555 100644 --- a/cmds/statsd/src/external/StatsPullerManager.h +++ b/cmds/statsd/src/external/StatsPullerManager.h @@ -16,9 +16,8 @@ #pragma once -#include -#include -#include +#include +#include #include #include @@ -30,11 +29,14 @@ #include "guardrail/StatsdStats.h" #include "logd/LogEvent.h" +using aidl::android::os::IPullAtomCallback; +using aidl::android::os::IStatsCompanionService; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { - typedef struct PullerKey { // The uid of the process that registers this puller. const int uid = -1; @@ -93,18 +95,18 @@ public: // Clear pull data cache if it is beyond respective cool down time. int ClearPullerCacheIfNecessary(int64_t timestampNs); - void SetStatsCompanionService(sp statsCompanionService); + void SetStatsCompanionService(shared_ptr statsCompanionService); void RegisterPullAtomCallback(const int uid, const int32_t atomTag, const int64_t coolDownNs, const int64_t timeoutNs, const vector& additiveFields, - const sp& callback); + const shared_ptr& callback); void UnregisterPullAtomCallback(const int uid, const int32_t atomTag); std::map> kAllPullAtomInfo; private: - sp mStatsCompanionService = nullptr; + shared_ptr mStatsCompanionService = nullptr; typedef struct { int64_t nextPullTimeNs; diff --git a/cmds/statsd/src/logd/LogEvent.cpp b/cmds/statsd/src/logd/LogEvent.cpp index 1ef1ab25098ca..9c508467dc8e0 100644 --- a/cmds/statsd/src/logd/LogEvent.cpp +++ b/cmds/statsd/src/logd/LogEvent.cpp @@ -20,7 +20,8 @@ #include "stats_log_util.h" #include "statslog.h" -#include +#include +#include #include namespace android { @@ -31,6 +32,7 @@ namespace statsd { const int FIELD_ID_EXPERIMENT_ID = 1; using namespace android::util; +using android::base::StringPrintf; using android::util::ProtoOutputStream; using std::string; using std::vector; @@ -180,8 +182,8 @@ LogEvent::LogEvent(const string& trainName, int64_t trainVersionCode, bool requi mLogdTimestampNs = getWallClockNs(); mElapsedTimestampNs = getElapsedRealtimeNs(); mTagId = android::util::BINARY_PUSH_STATE_CHANGED; - mLogUid = android::IPCThreadState::self()->getCallingUid(); - mLogPid = android::IPCThreadState::self()->getCallingPid(); + mLogUid = AIBinder_getCallingUid(); + mLogPid = AIBinder_getCallingPid(); mValues.push_back(FieldValue(Field(mTagId, getSimpleField(1)), Value(trainName))); mValues.push_back(FieldValue(Field(mTagId, getSimpleField(2)), Value(trainVersionCode))); diff --git a/cmds/statsd/src/main.cpp b/cmds/statsd/src/main.cpp index 140ef4e9cea9f..d79b6a21c19fa 100644 --- a/cmds/statsd/src/main.cpp +++ b/cmds/statsd/src/main.cpp @@ -20,9 +20,9 @@ #include "StatsService.h" #include "socket/StatsSocketListener.h" -#include -#include -#include +#include +#include +#include #include #include @@ -32,16 +32,11 @@ using namespace android; using namespace android::os::statsd; +using ::ndk::SharedRefBase; +using std::shared_ptr; +using std::make_shared; -/** - * Thread function data. - */ -struct log_reader_thread_data { - sp service; -}; - - -sp gStatsService = nullptr; +shared_ptr gStatsService = nullptr; void sigHandler(int sig) { if (gStatsService != nullptr) { @@ -68,20 +63,18 @@ int main(int /*argc*/, char** /*argv*/) { sp looper(Looper::prepare(0 /* opts */)); // Set up the binder - sp ps(ProcessState::self()); - ps->setThreadPoolMaxThreadCount(9); - ps->startThreadPool(); - ps->giveThreadPoolName(); - IPCThreadState::self()->disableBackgroundScheduling(true); + ABinderProcess_setThreadPoolMaxThreadCount(9); + ABinderProcess_startThreadPool(); std::shared_ptr eventQueue = std::make_shared(2000 /*buffer limit. Buffer is NOT pre-allocated*/); // Create the service - gStatsService = new StatsService(looper, eventQueue); - if (defaultServiceManager()->addService(String16("stats"), gStatsService, false, - IServiceManager::DUMP_FLAG_PRIORITY_NORMAL | IServiceManager::DUMP_FLAG_PROTO) - != 0) { + gStatsService = SharedRefBase::make(looper, eventQueue); + // TODO(b/149582373): Set DUMP_FLAG_PROTO once libbinder_ndk supports + // setting dumpsys priorities. + binder_status_t status = AServiceManager_addService(gStatsService->asBinder().get(), "stats"); + if (status != STATUS_OK) { ALOGE("Failed to add service as AIDL service"); return -1; } diff --git a/cmds/statsd/src/packages/UidMap.h b/cmds/statsd/src/packages/UidMap.h index bfac6e3431b08..4e3c506e18120 100644 --- a/cmds/statsd/src/packages/UidMap.h +++ b/cmds/statsd/src/packages/UidMap.h @@ -21,10 +21,11 @@ #include "packages/PackageInfoListener.h" #include "stats_util.h" -#include #include #include #include +#include + #include #include #include diff --git a/cmds/statsd/src/shell/ShellSubscriber.cpp b/cmds/statsd/src/shell/ShellSubscriber.cpp index d6a04336bc462..a861a3b76868e 100644 --- a/cmds/statsd/src/shell/ShellSubscriber.cpp +++ b/cmds/statsd/src/shell/ShellSubscriber.cpp @@ -29,49 +29,88 @@ namespace statsd { const static int FIELD_ID_ATOM = 1; -void ShellSubscriber::startNewSubscription(int in, int out, sp resultReceiver, - int timeoutSec) { +void ShellSubscriber::startNewSubscription(int in, int out, int timeoutSec) { VLOG("start new shell subscription"); + int64_t subscriberId = getElapsedRealtimeNs(); + { std::lock_guard lock(mMutex); - if (mResultReceiver != nullptr) { + if (mSubscriberId> 0) { VLOG("Only one shell subscriber is allowed."); return; } + mSubscriberId = subscriberId; mInput = in; mOutput = out; - mResultReceiver = resultReceiver; - IInterface::asBinder(mResultReceiver)->linkToDeath(this); } - // Note that the following is blocking, and it's intended as we cannot return until the shell - // cmd exits, otherwise all resources & FDs will be automatically closed. + bool success = readConfig(); + if (!success) { + std::lock_guard lock(mMutex); + cleanUpLocked(); + } - // Read config forever until EOF is reached. Clients may send multiple configs -- each new - // config replace the previous one. - readConfig(in); - VLOG("timeout : %d", timeoutSec); - - // Now we have read an EOF we now wait for the semaphore until the client exits. - VLOG("Now wait for client to exit"); + VLOG("Wait for client to exit or timeout (%d sec)", timeoutSec); std::unique_lock lk(mMutex); + // Note that the following is blocking, and it's intended as we cannot return until the shell + // cmd exits or we time out. if (timeoutSec > 0) { mShellDied.wait_for(lk, timeoutSec * 1s, - [this, resultReceiver] { return mResultReceiver != resultReceiver; }); + [this, subscriberId] { return mSubscriberId != subscriberId; }); } else { - mShellDied.wait(lk, [this, resultReceiver] { return mResultReceiver != resultReceiver; }); + mShellDied.wait(lk, [this, subscriberId] { return mSubscriberId != subscriberId; }); + } +} + + +// Read configs until EOF is reached. There may be multiple configs in the input +// -- each new config should replace the previous one. +// +// Returns a boolean indicating whether the input was read successfully. +bool ShellSubscriber::readConfig() { + if (mInput < 0) { + return false; + } + + while (true) { + // Read the size of the config. + size_t bufferSize = 0; + ssize_t bytesRead = read(mInput, &bufferSize, sizeof(bufferSize)); + if (bytesRead == 0) { + VLOG("We have reached the end of the input."); + return true; + } else if (bytesRead < 0 || (size_t)bytesRead != sizeof(bufferSize)) { + ALOGE("Error reading config size"); + return false; + } + + // Read and parse the config. + vector buffer(bufferSize); + bytesRead = read(mInput, buffer.data(), bufferSize); + if (bytesRead > 0 && (size_t)bytesRead == bufferSize) { + ShellSubscription config; + if (config.ParseFromArray(buffer.data(), bufferSize)) { + updateConfig(config); + } else { + ALOGE("Error parsing the config"); + return false; + } + } else { + VLOG("Error reading the config, expected bytes: %zu, actual bytes: %zu", bufferSize, + bytesRead); + return false; + } } } void ShellSubscriber::updateConfig(const ShellSubscription& config) { - std::lock_guard lock(mMutex); mPushedMatchers.clear(); mPulledInfo.clear(); for (const auto& pushed : config.pushed()) { mPushedMatchers.push_back(pushed); - VLOG("adding matcher for atom %d", pushed.atom_id()); + VLOG("adding matcher for pushed atom %d", pushed.atom_id()); } int64_t token = getElapsedRealtimeNs(); @@ -89,46 +128,20 @@ void ShellSubscriber::updateConfig(const ShellSubscription& config) { } if (mPulledInfo.size() > 0 && minInterval > 0) { - // This thread is guaranteed to terminate after it detects the token is different or - // cleaned up. + // This thread is guaranteed to terminate after it detects the token is + // different. std::thread puller([token, minInterval, this] { startPull(token, minInterval); }); puller.detach(); } } -void ShellSubscriber::writeToOutputLocked(const vector>& data, - const SimpleAtomMatcher& matcher) { - if (mOutput == 0) return; - int count = 0; - mProto.clear(); - for (const auto& event : data) { - VLOG("%s", event->ToString().c_str()); - if (matchesSimple(*mUidMap, matcher, *event)) { - VLOG("matched"); - count++; - uint64_t atomToken = mProto.start(util::FIELD_TYPE_MESSAGE | - util::FIELD_COUNT_REPEATED | FIELD_ID_ATOM); - event->ToProto(mProto); - mProto.end(atomToken); - } - } - - if (count > 0) { - // First write the payload size. - size_t bufferSize = mProto.size(); - write(mOutput, &bufferSize, sizeof(bufferSize)); - VLOG("%d atoms, proto size: %zu", count, bufferSize); - // Then write the payload. - mProto.flush(mOutput); - } - mProto.clear(); -} - void ShellSubscriber::startPull(int64_t token, int64_t intervalMillis) { - while (1) { + while (true) { int64_t nowMillis = getElapsedRealtimeMillis(); { std::lock_guard lock(mMutex); + // If the token has changed, the config has changed, so this + // puller can now stop. if (mPulledInfo.size() == 0 || mPullToken != token) { VLOG("Pulling thread %lld done!", (long long)token); return; @@ -152,55 +165,47 @@ void ShellSubscriber::startPull(int64_t token, int64_t intervalMillis) { } } -void ShellSubscriber::readConfig(int in) { - if (in <= 0) { +// Must be called with the lock acquired, so that mProto isn't being written to +// at the same time by multiple threads. +void ShellSubscriber::writeToOutputLocked(const vector>& data, + const SimpleAtomMatcher& matcher) { + if (mOutput < 0) { return; } - - while (1) { - size_t bufferSize = 0; - int result = 0; - if ((result = read(in, &bufferSize, sizeof(bufferSize))) == 0) { - VLOG("Done reading"); - break; - } else if (result < 0 || result != sizeof(bufferSize)) { - ALOGE("Error reading config size"); - break; - } - - vector buffer(bufferSize); - if ((result = read(in, buffer.data(), bufferSize)) > 0 && ((size_t)result) == bufferSize) { - ShellSubscription config; - if (config.ParseFromArray(buffer.data(), bufferSize)) { - updateConfig(config); - } else { - ALOGE("error parsing the config"); - break; - } - } else { - VLOG("Error reading the config, returned: %d, expecting %zu", result, bufferSize); - break; + int count = 0; + mProto.clear(); + for (const auto& event : data) { + VLOG("%s", event->ToString().c_str()); + if (matchesSimple(*mUidMap, matcher, *event)) { + VLOG("matched"); + count++; + uint64_t atomToken = mProto.start(util::FIELD_TYPE_MESSAGE | + util::FIELD_COUNT_REPEATED | FIELD_ID_ATOM); + event->ToProto(mProto); + mProto.end(atomToken); } } -} -void ShellSubscriber::cleanUpLocked() { - // The file descriptors will be closed by binder. - mInput = 0; - mOutput = 0; - mResultReceiver = nullptr; - mPushedMatchers.clear(); - mPulledInfo.clear(); - mPullToken = 0; - VLOG("done clean up"); + if (count > 0) { + // First write the payload size. + size_t bufferSize = mProto.size(); + write(mOutput, &bufferSize, sizeof(bufferSize)); + + VLOG("%d atoms, proto size: %zu", count, bufferSize); + // Then write the payload. + mProto.flush(mOutput); + } } void ShellSubscriber::onLogEvent(const LogEvent& event) { + // Acquire a lock to prevent corruption from multiple threads writing to + // mProto. std::lock_guard lock(mMutex); - - if (mOutput <= 0) { + if (mOutput < 0) { return; } + + mProto.clear(); for (const auto& matcher : mPushedMatchers) { if (matchesSimple(*mUidMap, matcher, event)) { VLOG("%s", event.ToString().c_str()); @@ -208,25 +213,27 @@ void ShellSubscriber::onLogEvent(const LogEvent& event) { util::FIELD_COUNT_REPEATED | FIELD_ID_ATOM); event.ToProto(mProto); mProto.end(atomToken); + // First write the payload size. size_t bufferSize = mProto.size(); write(mOutput, &bufferSize, sizeof(bufferSize)); // Then write the payload. mProto.flush(mOutput); - mProto.clear(); - break; } } } -void ShellSubscriber::binderDied(const wp& who) { - { - VLOG("Shell exits"); - std::lock_guard lock(mMutex); - cleanUpLocked(); - } - mShellDied.notify_all(); +void ShellSubscriber::cleanUpLocked() { + // The file descriptors will be closed by binder. + mInput = -1; + mOutput = -1; + mSubscriberId = 0; + mPushedMatchers.clear(); + mPulledInfo.clear(); + // Setting mPullToken == 0 tells pull thread that its work is done. + mPullToken = 0; + VLOG("done clean up"); } } // namespace statsd diff --git a/cmds/statsd/src/shell/ShellSubscriber.h b/cmds/statsd/src/shell/ShellSubscriber.h index 86d85901083a9..eaf2ad141c8e7 100644 --- a/cmds/statsd/src/shell/ShellSubscriber.h +++ b/cmds/statsd/src/shell/ShellSubscriber.h @@ -19,7 +19,6 @@ #include "logd/LogEvent.h" #include -#include #include #include #include @@ -56,7 +55,7 @@ namespace statsd { * Only one shell subscriber allowed at a time, because each shell subscriber blocks one thread * until it exits. */ -class ShellSubscriber : public virtual IBinder::DeathRecipient { +class ShellSubscriber : public virtual RefBase { public: ShellSubscriber(sp uidMap, sp pullerMgr) : mUidMap(uidMap), mPullerMgr(pullerMgr){}; @@ -64,10 +63,7 @@ public: /** * Start a new subscription. */ - void startNewSubscription(int inFd, int outFd, sp resultReceiver, - int timeoutSec); - - void binderDied(const wp& who); + void startNewSubscription(int inFd, int outFd, int timeoutSec); void onLogEvent(const LogEvent& event); @@ -80,7 +76,7 @@ private: int64_t mInterval; int64_t mPrevPullElapsedRealtimeMs; }; - void readConfig(int in); + bool readConfig(); void updateConfig(const ShellSubscription& config); @@ -101,16 +97,16 @@ private: std::condition_variable mShellDied; // semaphore for waiting until shell exits. - int mInput; // The input file descriptor + int mInput = -1; // The input file descriptor - int mOutput; // The output file descriptor - - sp mResultReceiver; + int mOutput = -1; // The output file descriptor std::vector mPushedMatchers; std::vector mPulledInfo; + int64_t mSubscriberId = 0; // A unique id to identify a subscriber. + int64_t mPullToken = 0; // A unique token to identify a puller thread. }; diff --git a/cmds/statsd/src/stats_log_util.cpp b/cmds/statsd/src/stats_log_util.cpp index 73f640e130a84..3d02ffb0db753 100644 --- a/cmds/statsd/src/stats_log_util.cpp +++ b/cmds/statsd/src/stats_log_util.cpp @@ -17,6 +17,7 @@ #include "hash.h" #include "stats_log_util.h" +#include #include #include #include @@ -35,6 +36,10 @@ using android::util::FIELD_TYPE_STRING; using android::util::FIELD_TYPE_UINT64; using android::util::ProtoOutputStream; +using aidl::android::os::IStatsCompanionService; +using std::shared_ptr; +using std::string; + namespace android { namespace os { namespace statsd { @@ -587,13 +592,13 @@ int64_t MillisToNano(const int64_t millis) { } bool checkPermissionForIds(const char* permission, pid_t pid, uid_t uid) { - sp scs = getStatsCompanionService(); + shared_ptr scs = getStatsCompanionService(); if (scs == nullptr) { return false; } bool success; - binder::Status status = scs->checkPermission(String16(permission), pid, uid, &success); + ::ndk::ScopedAStatus status = scs->checkPermission(string(permission), pid, uid, &success); if (!status.isOk()) { return false; } diff --git a/cmds/statsd/src/statscompanion_util.cpp b/cmds/statsd/src/statscompanion_util.cpp index d338827774f90..ce07ec0ea884e 100644 --- a/cmds/statsd/src/statscompanion_util.cpp +++ b/cmds/statsd/src/statscompanion_util.cpp @@ -18,26 +18,16 @@ #include "Log.h" #include "statscompanion_util.h" +#include +#include namespace android { namespace os { namespace statsd { -sp getStatsCompanionService() { - sp statsCompanion = nullptr; - // Get statscompanion service from service manager - static const sp sm(defaultServiceManager()); - if (statsCompanion == nullptr) { - if (sm != nullptr) { - const String16 name("statscompanion"); - statsCompanion = interface_cast(sm->checkService(name)); - if (statsCompanion == nullptr) { - ALOGW("statscompanion service unavailable!"); - return nullptr; - } - } - } - return statsCompanion; +shared_ptr getStatsCompanionService() { + ::ndk::SpAIBinder binder(AServiceManager_getService("statscompanion")); + return IStatsCompanionService::fromBinder(binder); } } // namespace statsd diff --git a/cmds/statsd/src/statscompanion_util.h b/cmds/statsd/src/statscompanion_util.h index dc4f283612144..e20c40bba1040 100644 --- a/cmds/statsd/src/statscompanion_util.h +++ b/cmds/statsd/src/statscompanion_util.h @@ -16,14 +16,17 @@ #pragma once -#include "StatsLogProcessor.h" +#include + +using aidl::android::os::IStatsCompanionService; +using std::shared_ptr; namespace android { namespace os { namespace statsd { /** Fetches and returns the StatsCompanionService. */ -sp getStatsCompanionService(); +shared_ptr getStatsCompanionService(); } // namespace statsd } // namespace os diff --git a/cmds/statsd/src/subscriber/SubscriberReporter.cpp b/cmds/statsd/src/subscriber/SubscriberReporter.cpp index 8fd6b46d07161..93af5e9372283 100644 --- a/cmds/statsd/src/subscriber/SubscriberReporter.cpp +++ b/cmds/statsd/src/subscriber/SubscriberReporter.cpp @@ -27,38 +27,52 @@ namespace statsd { using std::vector; -class BroadcastSubscriberDeathRecipient : public android::IBinder::DeathRecipient { - public: - BroadcastSubscriberDeathRecipient(const ConfigKey& configKey, int64_t subscriberId): - mConfigKey(configKey), - mSubscriberId(subscriberId) {} - ~BroadcastSubscriberDeathRecipient() override = default; - private: - ConfigKey mConfigKey; - int64_t mSubscriberId; +struct BroadcastSubscriberDeathCookie { + BroadcastSubscriberDeathCookie(const ConfigKey& configKey, int64_t subscriberId, + const shared_ptr& pir): + mConfigKey(configKey), + mSubscriberId(subscriberId), + mPir(pir) {} - void binderDied(const android::wp& who) override { - if (IInterface::asBinder(SubscriberReporter::getInstance().getBroadcastSubscriber( - mConfigKey, mSubscriberId)) == who.promote()) { - SubscriberReporter::getInstance().unsetBroadcastSubscriber(mConfigKey, mSubscriberId); - } - } + ConfigKey mConfigKey; + int64_t mSubscriberId; + shared_ptr mPir; }; +static void broadcastSubscriberDied(void* cookie) { + BroadcastSubscriberDeathCookie* cookie_ = (BroadcastSubscriberDeathCookie*)cookie; + ConfigKey configKey = cookie_->mConfigKey; + int64_t subscriberId = cookie_->mSubscriberId; + shared_ptr pir = cookie_->mPir; + + // TODO(b/149254662): Fix threading. This currently fails if a new pir gets + // set between the get and the unset. + if (SubscriberReporter::getInstance().getBroadcastSubscriber(configKey, subscriberId) == pir) { + SubscriberReporter::getInstance().unsetBroadcastSubscriber(configKey, subscriberId); + } + // The death recipient corresponding to this specific pir can never be + // triggered again, so free up resources. + delete cookie_; +} + +static ::ndk::ScopedAIBinder_DeathRecipient sBroadcastSubscriberDeathRecipient( + AIBinder_DeathRecipient_new(broadcastSubscriberDied)); + void SubscriberReporter::setBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId, - const sp& pir) { + const shared_ptr& pir) { VLOG("SubscriberReporter::setBroadcastSubscriber called."); - lock_guard lock(mLock); + lock_guard lock(mLock); mIntentMap[configKey][subscriberId] = pir; - IInterface::asBinder(pir)->linkToDeath( - new BroadcastSubscriberDeathRecipient(configKey, subscriberId)); + // TODO(b/149254662): Is it ok to call linkToDeath while holding a lock? + AIBinder_linkToDeath(pir->asBinder().get(), sBroadcastSubscriberDeathRecipient.get(), + new BroadcastSubscriberDeathCookie(configKey, subscriberId, pir)); } void SubscriberReporter::unsetBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId) { VLOG("SubscriberReporter::unsetBroadcastSubscriber called."); - lock_guard lock(mLock); + lock_guard lock(mLock); auto subscriberMapIt = mIntentMap.find(configKey); if (subscriberMapIt != mIntentMap.end()) { subscriberMapIt->second.erase(subscriberId); @@ -80,7 +94,7 @@ void SubscriberReporter::alertBroadcastSubscriber(const ConfigKey& configKey, // config id - the name of this config (for this particular uid) VLOG("SubscriberReporter::alertBroadcastSubscriber called."); - lock_guard lock(mLock); + lock_guard lock(mLock); if (!subscription.has_broadcast_subscriber_details() || !subscription.broadcast_subscriber_details().has_subscriber_id()) { @@ -89,10 +103,12 @@ void SubscriberReporter::alertBroadcastSubscriber(const ConfigKey& configKey, } int64_t subscriberId = subscription.broadcast_subscriber_details().subscriber_id(); - vector cookies; + // TODO(b/149254662): Is there a way to convert a RepeatedPtrField into a + // vector without copying? + vector cookies; cookies.reserve(subscription.broadcast_subscriber_details().cookie_size()); for (auto& cookie : subscription.broadcast_subscriber_details().cookie()) { - cookies.push_back(String16(cookie.c_str())); + cookies.push_back(cookie); } auto it1 = mIntentMap.find(configKey); @@ -109,10 +125,10 @@ void SubscriberReporter::alertBroadcastSubscriber(const ConfigKey& configKey, sendBroadcastLocked(it2->second, configKey, subscription, cookies, dimKey); } -void SubscriberReporter::sendBroadcastLocked(const sp& pir, +void SubscriberReporter::sendBroadcastLocked(const shared_ptr& pir, const ConfigKey& configKey, const Subscription& subscription, - const vector& cookies, + const vector& cookies, const MetricDimensionKey& dimKey) const { VLOG("SubscriberReporter::sendBroadcastLocked called."); pir->sendSubscriberBroadcast( @@ -124,9 +140,9 @@ void SubscriberReporter::sendBroadcastLocked(const sp& pir, dimKey.getDimensionKeyInWhat().toStatsDimensionsValueParcel()); } -sp SubscriberReporter::getBroadcastSubscriber(const ConfigKey& configKey, - int64_t subscriberId) { - lock_guard lock(mLock); +shared_ptr SubscriberReporter::getBroadcastSubscriber(const ConfigKey& configKey, + int64_t subscriberId) { + lock_guard lock(mLock); auto subscriberMapIt = mIntentMap.find(configKey); if (subscriberMapIt == mIntentMap.end()) { return nullptr; diff --git a/cmds/statsd/src/subscriber/SubscriberReporter.h b/cmds/statsd/src/subscriber/SubscriberReporter.h index 42599f5083137..0f97d397e3319 100644 --- a/cmds/statsd/src/subscriber/SubscriberReporter.h +++ b/cmds/statsd/src/subscriber/SubscriberReporter.h @@ -16,9 +16,9 @@ #pragma once -#include -#include +#include #include +#include #include "config/ConfigKey.h" #include "frameworks/base/cmds/statsd/src/statsd_config.pb.h" // subscription @@ -28,6 +28,13 @@ #include #include +using aidl::android::os::IPendingIntentRef; +using std::mutex; +using std::shared_ptr; +using std::string; +using std::unordered_map; +using std::vector; + namespace android { namespace os { namespace statsd { @@ -51,7 +58,7 @@ public: */ void setBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId, - const sp& pir); + const shared_ptr& pir); /** * Erases any intentSender information from the given (configKey, subscriberId) pair. @@ -67,28 +74,25 @@ public: const Subscription& subscription, const MetricDimensionKey& dimKey) const; - sp getBroadcastSubscriber(const ConfigKey& configKey, int64_t subscriberId); + shared_ptr getBroadcastSubscriber(const ConfigKey& configKey, + int64_t subscriberId); private: SubscriberReporter() {}; - mutable std::mutex mLock; - - /** Binder interface for communicating with StatsCompanionService. */ - sp mStatsCompanionService = nullptr; + mutable mutex mLock; /** Maps -> IPendingIntentRef (which represents a PendingIntent). */ - std::unordered_map>> mIntentMap; + unordered_map>> mIntentMap; /** * Sends a broadcast via the given intentSender (using mStatsCompanionService), along * with the information in the other parameters. */ - void sendBroadcastLocked(const sp& pir, + void sendBroadcastLocked(const shared_ptr& pir, const ConfigKey& configKey, const Subscription& subscription, - const std::vector& cookies, + const vector& cookies, const MetricDimensionKey& dimKey) const; }; diff --git a/cmds/statsd/tests/AlarmMonitor_test.cpp b/cmds/statsd/tests/AlarmMonitor_test.cpp index 1fccb35eccb57..edbf8b5e8ee6a 100644 --- a/cmds/statsd/tests/AlarmMonitor_test.cpp +++ b/cmds/statsd/tests/AlarmMonitor_test.cpp @@ -17,14 +17,16 @@ #include using namespace android::os::statsd; +using std::shared_ptr; #ifdef __ANDROID__ TEST(AlarmMonitor, popSoonerThan) { std::string emptyMetricId; std::string emptyDimensionId; unordered_set, SpHash> set; - AlarmMonitor am(2, [](const sp&, int64_t){}, - [](const sp&){}); + AlarmMonitor am(2, + [](const shared_ptr&, int64_t){}, + [](const shared_ptr&){}); set = am.popSoonerThan(5); EXPECT_TRUE(set.empty()); diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp index 69e11ed9b836f..3d7e5c9bf34e6 100644 --- a/cmds/statsd/tests/StatsLogProcessor_test.cpp +++ b/cmds/statsd/tests/StatsLogProcessor_test.cpp @@ -32,6 +32,8 @@ using namespace android; using namespace testing; +using ::ndk::SharedRefBase; +using std::shared_ptr; namespace android { namespace os { @@ -49,10 +51,12 @@ public: MockMetricsManager() : MetricsManager(ConfigKey(1, 12345), StatsdConfig(), 1000, 1000, new UidMap(), new StatsPullerManager(), - new AlarmMonitor(10, [](const sp&, int64_t) {}, - [](const sp&) {}), - new AlarmMonitor(10, [](const sp&, int64_t) {}, - [](const sp&) {})) { + new AlarmMonitor(10, + [](const shared_ptr&, int64_t) {}, + [](const shared_ptr&) {}), + new AlarmMonitor(10, + [](const shared_ptr&, int64_t) {}, + [](const shared_ptr&) {})) { } MOCK_METHOD0(byteSize, size_t()); @@ -1488,9 +1492,9 @@ TEST(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart) { metric2ActivationTrigger2->set_activation_type(ACTIVATE_IMMEDIATELY); // Send the config. - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); string serialized = config1.SerializeAsString(); - service.addConfigurationChecked(uid, configId, {serialized.begin(), serialized.end()}); + service->addConfigurationChecked(uid, configId, {serialized.begin(), serialized.end()}); // Make sure the config is stored on disk. Otherwise, we will not reset on system server death. StatsdConfig tmpConfig; @@ -1501,7 +1505,7 @@ TEST(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart) { // Metric 2 is not active. // Metric 3 is active. // {{{--------------------------------------------------------------------------- - sp processor = service.mProcessor; + sp processor = service->mProcessor; EXPECT_EQ(1, processor->mMetricsManagers.size()); auto it = processor->mMetricsManagers.find(cfgKey1); EXPECT_TRUE(it != processor->mMetricsManagers.end()); @@ -1592,7 +1596,7 @@ TEST(StatsLogProcessorTest, TestActivationsPersistAcrossSystemServerRestart) { EXPECT_TRUE(approximateSystemServerDeath < NS_PER_SEC + configAddedTimeNs); // System server dies. - service.binderDied(nullptr); + service->statsCompanionServiceDiedImpl(); // We should have a new metrics manager. Lets get it and ensure activation status is restored. // {{{--------------------------------------------------------------------------- diff --git a/cmds/statsd/tests/StatsService_test.cpp b/cmds/statsd/tests/StatsService_test.cpp index 7c00531d560b1..86f786ef4b152 100644 --- a/cmds/statsd/tests/StatsService_test.cpp +++ b/cmds/statsd/tests/StatsService_test.cpp @@ -16,6 +16,7 @@ #include "config/ConfigKey.h" #include "frameworks/base/cmds/statsd/src/statsd_config.pb.h" +#include #include #include @@ -29,33 +30,34 @@ namespace os { namespace statsd { using android::util::ProtoOutputStream; +using ::ndk::SharedRefBase; #ifdef __ANDROID__ TEST(StatsServiceTest, TestAddConfig_simple) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); StatsdConfig config; config.set_id(12345); string serialized = config.SerializeAsString(); EXPECT_TRUE( - service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); + service->addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); } TEST(StatsServiceTest, TestAddConfig_empty) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); string serialized = ""; EXPECT_TRUE( - service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); + service->addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); } TEST(StatsServiceTest, TestAddConfig_invalid) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); string serialized = "Invalid config!"; EXPECT_FALSE( - service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); + service->addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()})); } TEST(StatsServiceTest, TestGetUidFromArgs) { @@ -69,32 +71,32 @@ TEST(StatsServiceTest, TestGetUidFromArgs) { int32_t uid; - StatsService service(nullptr, nullptr); - service.mEngBuild = true; + shared_ptr service = SharedRefBase::make(nullptr, nullptr); + service->mEngBuild = true; // "-1" - EXPECT_FALSE(service.getUidFromArgs(args, 0, uid)); + EXPECT_FALSE(service->getUidFromArgs(args, 0, uid)); // "0" - EXPECT_TRUE(service.getUidFromArgs(args, 1, uid)); + EXPECT_TRUE(service->getUidFromArgs(args, 1, uid)); EXPECT_EQ(0, uid); // "1" - EXPECT_TRUE(service.getUidFromArgs(args, 2, uid)); + EXPECT_TRUE(service->getUidFromArgs(args, 2, uid)); EXPECT_EQ(1, uid); // "999999999999999999" - EXPECT_FALSE(service.getUidFromArgs(args, 3, uid)); + EXPECT_FALSE(service->getUidFromArgs(args, 3, uid)); // "a1" - EXPECT_FALSE(service.getUidFromArgs(args, 4, uid)); + EXPECT_FALSE(service->getUidFromArgs(args, 4, uid)); // "" - EXPECT_FALSE(service.getUidFromArgs(args, 5, uid)); + EXPECT_FALSE(service->getUidFromArgs(args, 5, uid)); // For a non-userdebug, uid "1" cannot be impersonated. - service.mEngBuild = false; - EXPECT_FALSE(service.getUidFromArgs(args, 2, uid)); + service->mEngBuild = false; + EXPECT_FALSE(service->getUidFromArgs(args, 2, uid)); } #else diff --git a/cmds/statsd/tests/anomaly/AlarmTracker_test.cpp b/cmds/statsd/tests/anomaly/AlarmTracker_test.cpp index 5e6de1cc6b91f..90ffcd009efde 100644 --- a/cmds/statsd/tests/anomaly/AlarmTracker_test.cpp +++ b/cmds/statsd/tests/anomaly/AlarmTracker_test.cpp @@ -15,12 +15,14 @@ #include "src/anomaly/AlarmTracker.h" #include +#include #include #include using namespace testing; using android::sp; using std::set; +using std::shared_ptr; using std::unordered_map; using std::vector; @@ -34,8 +36,9 @@ const ConfigKey kConfigKey(0, 12345); TEST(AlarmTrackerTest, TestTriggerTimestamp) { sp subscriberAlarmMonitor = - new AlarmMonitor(100, [](const sp&, int64_t){}, - [](const sp&){}); + new AlarmMonitor(100, + [](const shared_ptr&, int64_t){}, + [](const shared_ptr&){}); Alarm alarm; alarm.set_offset_millis(15 * MS_PER_SEC); alarm.set_period_millis(60 * 60 * MS_PER_SEC); // 1hr diff --git a/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp b/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp index 5e60abaf77923..9127be804dbd3 100644 --- a/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp +++ b/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include @@ -20,6 +21,8 @@ #include "src/stats_log_util.h" #include "tests/statsd_test_util.h" +using ::ndk::SharedRefBase; + namespace android { namespace os { namespace statsd { @@ -71,7 +74,8 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), ATOM_TAG); + SharedRefBase::make(), + ATOM_TAG); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); processor->mPullerManager->ForceClearPullerCache(); @@ -216,7 +220,8 @@ TEST(GaugeMetricE2eTest, TestConditionChangeToTrueSamplePulledEvents) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), ATOM_TAG); + SharedRefBase::make(), + ATOM_TAG); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); processor->mPullerManager->ForceClearPullerCache(); @@ -317,7 +322,8 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvent_LateAlarm) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback, ATOM_TAG); + SharedRefBase::make(), + ATOM_TAG); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); processor->mPullerManager->ForceClearPullerCache(); @@ -426,7 +432,8 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEventsWithActivation) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), ATOM_TAG); + SharedRefBase::make(), + ATOM_TAG); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); processor->mPullerManager->ForceClearPullerCache(); @@ -544,7 +551,8 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEventsNoCondition) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), ATOM_TAG); + SharedRefBase::make(), + ATOM_TAG); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); processor->mPullerManager->ForceClearPullerCache(); diff --git a/cmds/statsd/tests/e2e/PartialBucket_e2e_test.cpp b/cmds/statsd/tests/e2e/PartialBucket_e2e_test.cpp index 9d58867f09db4..1eecbe5dacafe 100644 --- a/cmds/statsd/tests/e2e/PartialBucket_e2e_test.cpp +++ b/cmds/statsd/tests/e2e/PartialBucket_e2e_test.cpp @@ -12,9 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include -#include -#include +#include +#include #include #include @@ -24,6 +23,9 @@ #include "src/stats_log_util.h" #include "tests/statsd_test_util.h" +using::ndk::SharedRefBase; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { @@ -34,19 +36,17 @@ const string kApp1 = "app1.sharing.1"; const int kConfigKey = 789130123; // Randomly chosen to avoid collisions with existing configs. const int kCallingUid = 0; // Randomly chosen -void SendConfig(StatsService& service, const StatsdConfig& config) { +void SendConfig(shared_ptr& service, const StatsdConfig& config) { string str; config.SerializeToString(&str); - std::vector configAsVec(str.begin(), str.end()); - bool success; - service.addConfiguration(kConfigKey, configAsVec, kCallingUid); + std::vector configAsVec(str.begin(), str.end()); + service->addConfiguration(kConfigKey, configAsVec, kCallingUid); } ConfigMetricsReport GetReports(sp processor, int64_t timestamp, bool include_current = false) { vector output; - IPCThreadState* ipc = IPCThreadState::self(); - ConfigKey configKey(ipc->getCallingUid(), kConfigKey); + ConfigKey configKey(AIBinder_getCallingUid(), kConfigKey); processor->onDumpReport(configKey, timestamp, include_current /* include_current_bucket*/, true /* erase_data */, ADB_DUMP, NO_TIME_CONSTRAINTS, &output); ConfigMetricsReportList reports; @@ -114,56 +114,56 @@ StatsdConfig MakeGaugeMetricConfig(int64_t minTime) { } // anonymous namespace TEST(PartialBucketE2eTest, TestCountMetricWithoutSplit) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); SendConfig(service, MakeConfig()); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 2).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 2).get()); - ConfigMetricsReport report = GetReports(service.mProcessor, start + 3); + ConfigMetricsReport report = GetReports(service->mProcessor, start + 3); // Expect no metrics since the bucket has not finished yet. EXPECT_EQ(1, report.metrics_size()); EXPECT_EQ(0, report.metrics(0).count_metrics().data_size()); } TEST(PartialBucketE2eTest, TestCountMetricNoSplitOnNewApp) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); SendConfig(service, MakeConfig()); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. // Force the uidmap to update at timestamp 2. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); // This is a new installation, so there shouldn't be a split (should be same as the without // split case). - service.mUidMap->updateApp(start + 2, String16(kApp1.c_str()), 1, 2, String16("v2"), + service->mUidMap->updateApp(start + 2, String16(kApp1.c_str()), 1, 2, String16("v2"), String16("")); // Goes into the second bucket. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); - ConfigMetricsReport report = GetReports(service.mProcessor, start + 4); + ConfigMetricsReport report = GetReports(service->mProcessor, start + 4); EXPECT_EQ(1, report.metrics_size()); EXPECT_EQ(0, report.metrics(0).count_metrics().data_size()); } TEST(PartialBucketE2eTest, TestCountMetricSplitOnUpgrade) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); SendConfig(service, MakeConfig()); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. - service.mUidMap->updateMap(start, {1}, {1}, {String16("v1")}, {String16(kApp1.c_str())}, - {String16("")}); + service->mUidMap->updateMap(start, {1}, {1}, {String16("v1")}, {String16(kApp1.c_str())}, + {String16("")}); // Force the uidmap to update at timestamp 2. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); - service.mUidMap->updateApp(start + 2, String16(kApp1.c_str()), 1, 2, String16("v2"), + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); + service->mUidMap->updateApp(start + 2, String16(kApp1.c_str()), 1, 2, String16("v2"), String16("")); // Goes into the second bucket. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); - ConfigMetricsReport report = GetReports(service.mProcessor, start + 4); + ConfigMetricsReport report = GetReports(service->mProcessor, start + 4); backfillStartEndTimestamp(&report); ASSERT_EQ(1, report.metrics_size()); @@ -177,20 +177,20 @@ TEST(PartialBucketE2eTest, TestCountMetricSplitOnUpgrade) { } TEST(PartialBucketE2eTest, TestCountMetricSplitOnRemoval) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); SendConfig(service, MakeConfig()); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. - service.mUidMap->updateMap(start, {1}, {1}, {String16("v1")}, {String16(kApp1.c_str())}, - {String16("")}); + service->mUidMap->updateMap(start, {1}, {1}, {String16("v1")}, {String16(kApp1.c_str())}, + {String16("")}); // Force the uidmap to update at timestamp 2. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); - service.mUidMap->removeApp(start + 2, String16(kApp1.c_str()), 1); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 1).get()); + service->mUidMap->removeApp(start + 2, String16(kApp1.c_str()), 1); // Goes into the second bucket. - service.mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); + service->mProcessor->OnLogEvent(CreateAppCrashEvent(100, start + 3).get()); - ConfigMetricsReport report = GetReports(service.mProcessor, start + 4); + ConfigMetricsReport report = GetReports(service->mProcessor, start + 4); backfillStartEndTimestamp(&report); ASSERT_EQ(1, report.metrics_size()); @@ -204,44 +204,44 @@ TEST(PartialBucketE2eTest, TestCountMetricSplitOnRemoval) { } TEST(PartialBucketE2eTest, TestValueMetricWithoutMinPartialBucket) { - StatsService service(nullptr, nullptr); - service.mPullerManager->RegisterPullAtomCallback( + shared_ptr service = SharedRefBase::make(nullptr, nullptr); + service->mPullerManager->RegisterPullAtomCallback( /*uid=*/0, android::util::SUBSYSTEM_SLEEP_STATE, NS_PER_SEC, NS_PER_SEC * 10, {}, - new FakeSubsystemSleepCallback()); + SharedRefBase::make()); // Partial buckets don't occur when app is first installed. - service.mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); + service->mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); SendConfig(service, MakeValueMetricConfig(0)); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. - service.mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); - service.mUidMap->updateApp(5 * 60 * NS_PER_SEC + start + 2, String16(kApp1.c_str()), 1, 2, - String16("v2"), String16("")); + service->mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); + service->mUidMap->updateApp(5 * 60 * NS_PER_SEC + start + 2, String16(kApp1.c_str()), 1, 2, + String16("v2"), String16("")); ConfigMetricsReport report = - GetReports(service.mProcessor, 5 * 60 * NS_PER_SEC + start + 100, true); + GetReports(service->mProcessor, 5 * 60 * NS_PER_SEC + start + 100, true); EXPECT_EQ(1, report.metrics_size()); EXPECT_EQ(0, report.metrics(0).value_metrics().skipped_size()); } TEST(PartialBucketE2eTest, TestValueMetricWithMinPartialBucket) { - StatsService service(nullptr, nullptr); - service.mPullerManager->RegisterPullAtomCallback( + shared_ptr service = SharedRefBase::make(nullptr, nullptr); + service->mPullerManager->RegisterPullAtomCallback( /*uid=*/0, android::util::SUBSYSTEM_SLEEP_STATE, NS_PER_SEC, NS_PER_SEC * 10, {}, - new FakeSubsystemSleepCallback()); + SharedRefBase::make()); // Partial buckets don't occur when app is first installed. - service.mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); + service->mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); SendConfig(service, MakeValueMetricConfig(60 * NS_PER_SEC /* One minute */)); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. const int64_t endSkipped = 5 * 60 * NS_PER_SEC + start + 2; - service.mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); - service.mUidMap->updateApp(endSkipped, String16(kApp1.c_str()), 1, 2, String16("v2"), + service->mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); + service->mUidMap->updateApp(endSkipped, String16(kApp1.c_str()), 1, 2, String16("v2"), String16("")); ConfigMetricsReport report = - GetReports(service.mProcessor, 5 * 60 * NS_PER_SEC + start + 100 * NS_PER_SEC, true); + GetReports(service->mProcessor, 5 * 60 * NS_PER_SEC + start + 100 * NS_PER_SEC, true); backfillStartEndTimestamp(&report); ASSERT_EQ(1, report.metrics_size()); @@ -253,38 +253,38 @@ TEST(PartialBucketE2eTest, TestValueMetricWithMinPartialBucket) { } TEST(PartialBucketE2eTest, TestGaugeMetricWithoutMinPartialBucket) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); // Partial buckets don't occur when app is first installed. - service.mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); + service->mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); SendConfig(service, MakeGaugeMetricConfig(0)); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. - service.mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); - service.mUidMap->updateApp(5 * 60 * NS_PER_SEC + start + 2, String16(kApp1.c_str()), 1, 2, + service->mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); + service->mUidMap->updateApp(5 * 60 * NS_PER_SEC + start + 2, String16(kApp1.c_str()), 1, 2, String16("v2"), String16("")); ConfigMetricsReport report = - GetReports(service.mProcessor, 5 * 60 * NS_PER_SEC + start + 100, true); + GetReports(service->mProcessor, 5 * 60 * NS_PER_SEC + start + 100, true); EXPECT_EQ(1, report.metrics_size()); EXPECT_EQ(0, report.metrics(0).gauge_metrics().skipped_size()); } TEST(PartialBucketE2eTest, TestGaugeMetricWithMinPartialBucket) { - StatsService service(nullptr, nullptr); + shared_ptr service = SharedRefBase::make(nullptr, nullptr); // Partial buckets don't occur when app is first installed. - service.mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); + service->mUidMap->updateApp(1, String16(kApp1.c_str()), 1, 1, String16("v1"), String16("")); SendConfig(service, MakeGaugeMetricConfig(60 * NS_PER_SEC /* One minute */)); int64_t start = getElapsedRealtimeNs(); // This is the start-time the metrics producers are // initialized with. const int64_t endSkipped = 5 * 60 * NS_PER_SEC + start + 2; - service.mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); - service.mUidMap->updateApp(endSkipped, String16(kApp1.c_str()), 1, 2, String16("v2"), - String16("")); + service->mProcessor->informPullAlarmFired(5 * 60 * NS_PER_SEC + start); + service->mUidMap->updateApp(endSkipped, String16(kApp1.c_str()), 1, 2, String16("v2"), + String16("")); ConfigMetricsReport report = - GetReports(service.mProcessor, 5 * 60 * NS_PER_SEC + start + 100 * NS_PER_SEC, true); + GetReports(service->mProcessor, 5 * 60 * NS_PER_SEC + start + 100 * NS_PER_SEC, true); backfillStartEndTimestamp(&report); ASSERT_EQ(1, report.metrics_size()); ASSERT_EQ(1, report.metrics(0).gauge_metrics().skipped_size()); diff --git a/cmds/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp b/cmds/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp index a140af8764748..9d39f9c7791ae 100644 --- a/cmds/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp +++ b/cmds/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include "src/StatsLogProcessor.h" @@ -20,6 +21,8 @@ #include +using ::ndk::SharedRefBase; + namespace android { namespace os { namespace statsd { @@ -70,7 +73,7 @@ TEST(ValueMetricE2eTest, TestPulledEvents) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), + SharedRefBase::make(), android::util::SUBSYSTEM_SLEEP_STATE); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); @@ -175,7 +178,7 @@ TEST(ValueMetricE2eTest, TestPulledEvents_LateAlarm) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), + SharedRefBase::make(), android::util::SUBSYSTEM_SLEEP_STATE); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); @@ -288,7 +291,7 @@ TEST(ValueMetricE2eTest, TestPulledEvents_WithActivation) { ConfigKey cfgKey; auto processor = CreateStatsLogProcessor(baseTimeNs, configAddedTimeNs, config, cfgKey, - new FakeSubsystemSleepCallback(), + SharedRefBase::make(), android::util::SUBSYSTEM_SLEEP_STATE); EXPECT_EQ(processor->mMetricsManagers.size(), 1u); EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid()); diff --git a/cmds/statsd/tests/external/StatsCallbackPuller_test.cpp b/cmds/statsd/tests/external/StatsCallbackPuller_test.cpp index a011692ee625f..e416b4c0e29b9 100644 --- a/cmds/statsd/tests/external/StatsCallbackPuller_test.cpp +++ b/cmds/statsd/tests/external/StatsCallbackPuller_test.cpp @@ -14,8 +14,10 @@ #include "src/external/StatsCallbackPuller.h" -#include -#include +#include +#include +#include +#include #include #include #include @@ -35,6 +37,11 @@ namespace os { namespace statsd { using namespace testing; +using Status = ::ndk::ScopedAStatus; +using aidl::android::os::BnPullAtomCallback; +using aidl::android::os::IPullAtomResultReceiver; +using aidl::android::util::StatsEventParcel; +using ::ndk::SharedRefBase; using std::make_shared; using std::shared_ptr; using std::vector; @@ -58,15 +65,15 @@ AStatsEvent* createSimpleEvent(int64_t value) { return event; } -void executePull(const sp& resultReceiver) { +void executePull(const shared_ptr& resultReceiver) { // Convert stats_events into StatsEventParcels. - std::vector parcels; + vector parcels; for (int i = 0; i < values.size(); i++) { AStatsEvent* event = createSimpleEvent(values[i]); size_t size; uint8_t* buffer = AStatsEvent_getBuffer(event, &size); - android::util::StatsEventParcel p; + StatsEventParcel p; // vector.assign() creates a copy, but this is inevitable unless // stats_event.h/c uses a vector as opposed to a buffer. p.buffer.assign(buffer, buffer + size); @@ -80,11 +87,11 @@ void executePull(const sp& resultReceiver) { class FakePullAtomCallback : public BnPullAtomCallback { public: - binder::Status onPullAtom(int atomTag, - const sp& resultReceiver) override { + Status onPullAtom(int atomTag, + const shared_ptr& resultReceiver) override { // Force pull to happen in separate thread to simulate binder. pullThread = std::thread(executePull, resultReceiver); - return binder::Status::ok(); + return Status::ok(); } }; @@ -111,7 +118,7 @@ public: } // Anonymous namespace. TEST_F(StatsCallbackPullerTest, PullSuccess) { - sp cb = new FakePullAtomCallback(); + shared_ptr cb = SharedRefBase::make(); int64_t value = 43; pullSuccess = true; values.push_back(value); @@ -132,20 +139,20 @@ TEST_F(StatsCallbackPullerTest, PullSuccess) { } TEST_F(StatsCallbackPullerTest, PullFail) { - sp cb = new FakePullAtomCallback(); + shared_ptr cb = SharedRefBase::make(); pullSuccess = false; int64_t value = 1234; values.push_back(value); StatsCallbackPuller puller(pullTagId, cb, pullCoolDownNs, pullTimeoutNs, {}); - vector> dataHolder; + vector> dataHolder; EXPECT_FALSE(puller.PullInternal(&dataHolder)); EXPECT_EQ(0, dataHolder.size()); } TEST_F(StatsCallbackPullerTest, PullTimeout) { - sp cb = new FakePullAtomCallback(); + shared_ptr cb = SharedRefBase::make(); pullSuccess = true; pullDelayNs = 500000000; // 500ms. pullTimeoutNs = 10000; // 10 microseconds. @@ -154,7 +161,7 @@ TEST_F(StatsCallbackPullerTest, PullTimeout) { StatsCallbackPuller puller(pullTagId, cb, pullCoolDownNs, pullTimeoutNs, {}); - vector> dataHolder; + vector> dataHolder; int64_t startTimeNs = getElapsedRealtimeNs(); // Returns true to let StatsPuller code evaluate the timeout. EXPECT_TRUE(puller.PullInternal(&dataHolder)); @@ -174,7 +181,7 @@ TEST_F(StatsCallbackPullerTest, PullTimeout) { // Register a puller and ensure that the timeout logic works. TEST_F(StatsCallbackPullerTest, RegisterAndTimeout) { - sp cb = new FakePullAtomCallback(); + shared_ptr cb = SharedRefBase::make(); pullSuccess = true; pullDelayNs = 500000000; // 500 ms. pullTimeoutNs = 10000; // 10 microsseconds. @@ -184,7 +191,7 @@ TEST_F(StatsCallbackPullerTest, RegisterAndTimeout) { StatsPullerManager pullerManager; pullerManager.RegisterPullAtomCallback(/*uid=*/-1, pullTagId, pullCoolDownNs, pullTimeoutNs, vector(), cb); - vector> dataHolder; + vector> dataHolder; int64_t startTimeNs = getElapsedRealtimeNs(); // Returns false, since StatsPuller code will evaluate the timeout. EXPECT_FALSE(pullerManager.Pull(pullTagId, &dataHolder)); diff --git a/cmds/statsd/tests/shell/ShellSubscriber_test.cpp b/cmds/statsd/tests/shell/ShellSubscriber_test.cpp index 73d1fd7850e94..dac5f33937d9f 100644 --- a/cmds/statsd/tests/shell/ShellSubscriber_test.cpp +++ b/cmds/statsd/tests/shell/ShellSubscriber_test.cpp @@ -34,27 +34,6 @@ using testing::StrictMock; #ifdef __ANDROID__ -class MyResultReceiver : public BnResultReceiver { -public: - Mutex mMutex; - Condition mCondition; - bool mHaveResult = false; - int32_t mResult = 0; - - virtual void send(int32_t resultCode) { - AutoMutex _l(mMutex); - mResult = resultCode; - mHaveResult = true; - mCondition.signal(); - } - - int32_t waitForResult() { - AutoMutex _l(mMutex); - mCondition.waitRelative(mMutex, 1000000000); - return mResult; - } -}; - void runShellTest(ShellSubscription config, sp uidMap, sp pullerManager, const vector>& pushedEvents, @@ -67,10 +46,7 @@ void runShellTest(ShellSubscription config, sp uidMap, ASSERT_EQ(0, pipe(fds_data)); size_t bufferSize = config.ByteSize(); - // write the config to pipe, first write size of the config - vector size_buffer(sizeof(bufferSize)); - std::memcpy(size_buffer.data(), &bufferSize, sizeof(bufferSize)); write(fds_config[1], &bufferSize, sizeof(bufferSize)); // then write config itself vector buffer(bufferSize); @@ -79,11 +55,10 @@ void runShellTest(ShellSubscription config, sp uidMap, close(fds_config[1]); sp shellClient = new ShellSubscriber(uidMap, pullerManager); - sp resultReceiver = new MyResultReceiver(); // mimic a binder thread that a shell subscriber runs on. it would block. - std::thread reader([&resultReceiver, &fds_config, &fds_data, &shellClient] { - shellClient->startNewSubscription(fds_config[0], fds_data[1], resultReceiver, -1); + std::thread reader([&shellClient, &fds_config, &fds_data] { + shellClient->startNewSubscription(fds_config[0], fds_data[1], /*timeoutSec=*/-1); }); reader.detach(); diff --git a/cmds/statsd/tests/statsd_test_util.cpp b/cmds/statsd/tests/statsd_test_util.cpp index db09ee9311ba9..69582184f1057 100644 --- a/cmds/statsd/tests/statsd_test_util.cpp +++ b/cmds/statsd/tests/statsd_test_util.cpp @@ -14,6 +14,11 @@ #include "statsd_test_util.h" +#include + +using aidl::android::util::StatsEventParcel; +using std::shared_ptr; + namespace android { namespace os { namespace statsd { @@ -581,7 +586,7 @@ std::unique_ptr CreateUidProcessStateChangedEvent( sp CreateStatsLogProcessor(const int64_t timeBaseNs, const int64_t currentTimeNs, const StatsdConfig& config, const ConfigKey& key, - const sp& puller, + const shared_ptr& puller, const int32_t atomTag) { sp uidMap = new UidMap(); sp pullerManager = new StatsPullerManager(); @@ -590,11 +595,13 @@ sp CreateStatsLogProcessor(const int64_t timeBaseNs, const in puller); } sp anomalyAlarmMonitor = - new AlarmMonitor(1, [](const sp&, int64_t){}, - [](const sp&){}); + new AlarmMonitor(1, + [](const shared_ptr&, int64_t){}, + [](const shared_ptr&){}); sp periodicAlarmMonitor = - new AlarmMonitor(1, [](const sp&, int64_t){}, - [](const sp&){}); + new AlarmMonitor(1, + [](const shared_ptr&, int64_t){}, + [](const shared_ptr&){}); sp processor = new StatsLogProcessor(uidMap, pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor, timeBaseNs, [](const ConfigKey&) { return true; }, @@ -948,10 +955,10 @@ void backfillStartEndTimestamp(ConfigMetricsReportList *config_report_list) { } } -binder::Status FakeSubsystemSleepCallback::onPullAtom( - int atomTag, const sp& resultReceiver) { +Status FakeSubsystemSleepCallback::onPullAtom(int atomTag, + const shared_ptr& resultReceiver) { // Convert stats_events into StatsEventParcels. - std::vector parcels; + std::vector parcels; for (int i = 1; i < 3; i++) { AStatsEvent* event = AStatsEvent_obtain(); AStatsEvent_setAtomId(event, atomTag); @@ -965,7 +972,7 @@ binder::Status FakeSubsystemSleepCallback::onPullAtom( size_t size; uint8_t* buffer = AStatsEvent_getBuffer(event, &size); - android::util::StatsEventParcel p; + StatsEventParcel p; // vector.assign() creates a copy, but this is inevitable unless // stats_event.h/c uses a vector as opposed to a buffer. p.buffer.assign(buffer, buffer + size); @@ -973,7 +980,7 @@ binder::Status FakeSubsystemSleepCallback::onPullAtom( AStatsEvent_write(event); } resultReceiver->pullFinished(atomTag, /*success=*/true, parcels); - return binder::Status::ok(); + return Status::ok(); } } // namespace statsd diff --git a/cmds/statsd/tests/statsd_test_util.h b/cmds/statsd/tests/statsd_test_util.h index 576a4916d60b2..c8326eef5698a 100644 --- a/cmds/statsd/tests/statsd_test_util.h +++ b/cmds/statsd/tests/statsd_test_util.h @@ -14,9 +14,9 @@ #pragma once -#include -#include -#include +#include +#include +#include #include #include "frameworks/base/cmds/statsd/src/stats_log.pb.h" @@ -31,8 +31,12 @@ namespace android { namespace os { namespace statsd { +using ::aidl::android::os::BnPullAtomCallback; +using ::aidl::android::os::IPullAtomCallback; +using ::aidl::android::os::IPullAtomResultReceiver; using android::util::ProtoReader; using google::protobuf::RepeatedPtrField; +using Status = ::ndk::ScopedAStatus; const int SCREEN_STATE_ATOM_ID = android::util::SCREEN_STATE_CHANGED; const int UID_PROCESS_STATE_ATOM_ID = android::util::UID_PROCESS_STATE_CHANGED; @@ -230,7 +234,7 @@ AttributionNodeInternal CreateAttribution(const int& uid, const string& tag); // Create a statsd log event processor upon the start time in seconds, config and key. sp CreateStatsLogProcessor(const int64_t timeBaseNs, const int64_t currentTimeNs, const StatsdConfig& config, const ConfigKey& key, - const sp& puller = nullptr, + const shared_ptr& puller = nullptr, const int32_t atomTag = 0 /*for puller only*/); // Util function to sort the log events by timestamp. @@ -285,8 +289,8 @@ bool backfillDimensionPath(const DimensionsValue& path, class FakeSubsystemSleepCallback : public BnPullAtomCallback { public: - binder::Status onPullAtom(int atomTag, - const sp& resultReceiver) override; + Status onPullAtom(int atomTag, + const shared_ptr& resultReceiver) override; }; template