Merge "Fix TODOs made during migration to libbinder_ndk" into rvc-dev am: 70d266df9b am: 794debebe7 (a2a11a28) · Commits · e / os / android_frameworks_base

cmds/statsd/src/StatsService.cpp

+0 −2

Original line number	Diff line number	Diff line
		@@ -930,8 +930,6 @@ Status StatsService::informOnePackage(const string& app, int32_t uid, int64_t ve
		ENFORCE_UID(AID_SYSTEM);

		VLOG("StatsService::informOnePackage was called");
		// 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());

cmds/statsd/src/anomaly/AlarmMonitor.cpp

+0 −2

Original line number	Diff line number	Diff line
		@@ -38,8 +38,6 @@ AlarmMonitor::~AlarmMonitor() {}
		void AlarmMonitor::setStatsCompanionService(
		shared_ptr<IStatsCompanionService> statsCompanionService) {
		std::lock_guard<std::mutex> lock(mLock);
		// TODO(b/149254662): determine if tmpForLock is needed now that we have moved
		// from sp to shared_ptr
		shared_ptr<IStatsCompanionService> tmpForLock = mStatsCompanionService;
		mStatsCompanionService = statsCompanionService;
		if (statsCompanionService == nullptr) {

cmds/statsd/src/config/ConfigManager.cpp

+31 −24

Original line number	Diff line number	Diff line
		@@ -54,20 +54,22 @@ struct ConfigReceiverDeathCookie {
		shared_ptr<IPendingIntentRef> mPir;
		};

		static void configReceiverDied(void* cookie) {
		void ConfigManager::configReceiverDied(void* cookie) {
		auto cookie_ = static_cast<ConfigReceiverDeathCookie*>(cookie);
		sp<ConfigManager> configManager = cookie_->mConfigManager;
		ConfigKey configKey = cookie_->mConfigKey;
		shared_ptr<IPendingIntentRef> pir = cookie_->mPir;
		sp<ConfigManager>& thiz = cookie_->mConfigManager;
		ConfigKey& configKey = cookie_->mConfigKey;
		shared_ptr<IPendingIntentRef>& 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->GetConfigReceiver(configKey) == pir) {
		configManager->RemoveConfigReceiver(configKey);
		// Erase the mapping from the config key to the config receiver (pir) if the
		// mapping still exists.
		lock_guard<mutex> lock(thiz->mMutex);
		auto it = thiz->mConfigReceivers.find(configKey);
		if (it != thiz->mConfigReceivers.end() && it->second == pir) {
		thiz->mConfigReceivers.erase(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.

		// The death recipient corresponding to this specific pir can never be
		// triggered again, so free up resources.
		delete cookie_;
		}

		@@ -83,17 +85,20 @@ struct ActiveConfigChangedReceiverDeathCookie {
		shared_ptr<IPendingIntentRef> mPir;
		};

		static void activeConfigChangedReceiverDied(void* cookie) {
		void ConfigManager::activeConfigChangedReceiverDied(void* cookie) {
		auto cookie_ = static_cast<ActiveConfigChangedReceiverDeathCookie*>(cookie);
		sp<ConfigManager> configManager = cookie_->mConfigManager;
		sp<ConfigManager>& thiz = cookie_->mConfigManager;
		int uid = cookie_->mUid;
		shared_ptr<IPendingIntentRef> pir = cookie_->mPir;
		shared_ptr<IPendingIntentRef>& 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);
		// Erase the mapping from the config key to the active config changed
		// receiver (pir) if the mapping still exists.
		lock_guard<mutex> lock(thiz->mMutex);
		auto it = thiz->mActiveConfigsChangedReceivers.find(uid);
		if (it != thiz->mActiveConfigsChangedReceivers.end() && it->second == pir) {
		thiz->mActiveConfigsChangedReceivers.erase(uid);
		}

		// The death recipient corresponding to this specific pir can never
		// be triggered again, so free up resources.
		delete cookie_;
		@@ -189,8 +194,10 @@ void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) {

		void ConfigManager::SetActiveConfigsChangedReceiver(const int uid,
		const shared_ptr<IPendingIntentRef>& pir) {
		{
		lock_guard<mutex> lock(mMutex);
		mActiveConfigsChangedReceivers[uid] = pir;
		}
		AIBinder_linkToDeath(pir->asBinder().get(), mActiveConfigChangedReceiverDeathRecipient.get(),
		new ActiveConfigChangedReceiverDeathCookie(this, uid, pir));
		}

cmds/statsd/src/config/ConfigManager.h

+13 −0

Original line number	Diff line number	Diff line
		@@ -161,6 +161,19 @@ private:
		// IPendingIntentRef dies.
		::ndk::ScopedAIBinder_DeathRecipient mConfigReceiverDeathRecipient;
		::ndk::ScopedAIBinder_DeathRecipient mActiveConfigChangedReceiverDeathRecipient;

		/**
		* Death recipient callback that is called when a config receiver dies.
		* The cookie is a pointer to a ConfigReceiverDeathCookie.
		*/
		static void configReceiverDied(void* cookie);

		/**
		* Death recipient callback that is called when an active config changed
		* receiver dies. The cookie is a pointer to an
		* ActiveConfigChangedReceiverDeathCookie.
		*/
		static void activeConfigChangedReceiverDied(void* cookie);
		};

		} // namespace statsd

cmds/statsd/src/external/StatsPullerManager.cpp

+3 −8

Original line number	Diff line number	Diff line
		@@ -85,12 +85,9 @@ void StatsPullerManager::updateAlarmLocked() {
		return;
		}

		// 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<IStatsCompanionService> statsCompanionServiceCopy = mStatsCompanionService;
		if (statsCompanionServiceCopy != nullptr) {
		statsCompanionServiceCopy->setPullingAlarm(mNextPullTimeNs / 1000000);
		// TODO(b/151045771): do not hold a lock while making a binder call
		if (mStatsCompanionService != nullptr) {
		mStatsCompanionService->setPullingAlarm(mNextPullTimeNs / 1000000);
		} else {
		VLOG("StatsCompanionService not available. Alarm not set.");
		}
		@@ -99,8 +96,6 @@ void StatsPullerManager::updateAlarmLocked() {

		void StatsPullerManager::SetStatsCompanionService(
		shared_ptr<IStatsCompanionService> 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);
		shared_ptr<IStatsCompanionService> tmpForLock = mStatsCompanionService;
		mStatsCompanionService = statsCompanionService;