[Statsd] Fix binder cookie memorie leak
Statsd was only deleting cookies for binder deaths when the death
notifier was invoked. However, it is possible for the binder to get
overwritten and for the client process to never die.
The fix for now is to not call LinkToDeath and avoid cookies entirely.
We lazily remove objects from the maps when we receive a dead object
exception on them.
Other options and more details are at go/statsd-binder-cookie-leak.
Test: atest statsd_test
Bug: 180471118
Bug: 202499776
Change-Id: Ie6ee3176e73a8602ca906396c0afc7df332deed1
diff --git a/statsd/src/config/ConfigManager.cpp b/statsd/src/config/ConfigManager.cpp
index 13020e0..d839e02 100644
--- a/statsd/src/config/ConfigManager.cpp
+++ b/statsd/src/config/ConfigManager.cpp
@@ -42,78 +42,7 @@
using android::base::StringPrintf;
using std::unique_ptr;
-struct ConfigReceiverDeathCookie {
- ConfigReceiverDeathCookie(const wp<ConfigManager>& configManager, const ConfigKey& configKey,
- const shared_ptr<IPendingIntentRef>& pir) :
- mConfigManager(configManager), mConfigKey(configKey), mPir(pir) {
- }
-
- wp<ConfigManager> mConfigManager;
- ConfigKey mConfigKey;
- shared_ptr<IPendingIntentRef> mPir;
-};
-
-void ConfigManager::configReceiverDied(void* cookie) {
- auto cookie_ = static_cast<ConfigReceiverDeathCookie*>(cookie);
- sp<ConfigManager> thiz = cookie_->mConfigManager.promote();
- if (!thiz) {
- return;
- }
-
- ConfigKey& configKey = cookie_->mConfigKey;
- shared_ptr<IPendingIntentRef>& pir = cookie_->mPir;
-
- // 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.
- delete cookie_;
-}
-
-struct ActiveConfigChangedReceiverDeathCookie {
- ActiveConfigChangedReceiverDeathCookie(const wp<ConfigManager>& configManager, const int uid,
- const shared_ptr<IPendingIntentRef>& pir) :
- mConfigManager(configManager), mUid(uid), mPir(pir) {
- }
-
- wp<ConfigManager> mConfigManager;
- int mUid;
- shared_ptr<IPendingIntentRef> mPir;
-};
-
-void ConfigManager::activeConfigChangedReceiverDied(void* cookie) {
- auto cookie_ = static_cast<ActiveConfigChangedReceiverDeathCookie*>(cookie);
- sp<ConfigManager> thiz = cookie_->mConfigManager.promote();
- if (!thiz) {
- return;
- }
-
- int uid = cookie_->mUid;
- shared_ptr<IPendingIntentRef>& pir = cookie_->mPir;
-
- // 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_;
-}
-
-ConfigManager::ConfigManager() :
- mConfigReceiverDeathRecipient(AIBinder_DeathRecipient_new(configReceiverDied)),
- mActiveConfigChangedReceiverDeathRecipient(
- AIBinder_DeathRecipient_new(activeConfigChangedReceiverDied)) {
+ConfigManager::ConfigManager() {
}
ConfigManager::~ConfigManager() {
@@ -189,8 +118,6 @@
const shared_ptr<IPendingIntentRef>& pir) {
lock_guard<mutex> lock(mMutex);
mConfigReceivers[key] = pir;
- AIBinder_linkToDeath(pir->asBinder().get(), mConfigReceiverDeathRecipient.get(),
- new ConfigReceiverDeathCookie(this, key, pir));
}
void ConfigManager::RemoveConfigReceiver(const ConfigKey& key) {
@@ -198,14 +125,19 @@
mConfigReceivers.erase(key);
}
+void ConfigManager::RemoveConfigReceiver(const ConfigKey& key,
+ const shared_ptr<IPendingIntentRef>& pir) {
+ lock_guard<mutex> lock(mMutex);
+ auto it = mConfigReceivers.find(key);
+ if (it != mConfigReceivers.end() && it->second == pir) {
+ mConfigReceivers.erase(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));
+ lock_guard<mutex> lock(mMutex);
+ mActiveConfigsChangedReceivers[uid] = pir;
}
void ConfigManager::RemoveActiveConfigsChangedReceiver(const int uid) {
@@ -213,6 +145,15 @@
mActiveConfigsChangedReceivers.erase(uid);
}
+void ConfigManager::RemoveActiveConfigsChangedReceiver(const int uid,
+ const shared_ptr<IPendingIntentRef>& pir) {
+ lock_guard<mutex> lock(mMutex);
+ auto it = mActiveConfigsChangedReceivers.find(uid);
+ if (it != mActiveConfigsChangedReceivers.end() && it->second == pir) {
+ mActiveConfigsChangedReceivers.erase(uid);
+ }
+}
+
void ConfigManager::RemoveConfig(const ConfigKey& key) {
vector<sp<ConfigListener>> broadcastList;
{
