Merge "Remove invalid configs from memory" into rvc-dev

4 files changed, 40 insertions, 0 deletions

diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index 095dd1e9be59..e7b32c56551a 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -529,7 +529,9 @@ void StatsLogProcessor::OnConfigUpdatedLocked(
         VLOG("StatsdConfig valid");
     } else {
         // If there is any error in the config, don't use it.
+        // Remove any existing config with the same key.
         ALOGE("StatsdConfig NOT valid");
+        mMetricsManagers.erase(key);
     }
 }
 
diff --git a/cmds/statsd/src/StatsLogProcessor.h b/cmds/statsd/src/StatsLogProcessor.h
index 7090bd46635d..23f2584655b0 100644
--- a/cmds/statsd/src/StatsLogProcessor.h
+++ b/cmds/statsd/src/StatsLogProcessor.h
@@ -284,6 +284,7 @@ private:
     FRIEND_TEST(StatsLogProcessorTest, TestRateLimitByteSize);
     FRIEND_TEST(StatsLogProcessorTest, TestRateLimitBroadcast);
     FRIEND_TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge);
+    FRIEND_TEST(StatsLogProcessorTest, InvalidConfigRemoved);
     FRIEND_TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead);
     FRIEND_TEST(StatsLogProcessorTest, TestActivationOnBoot);
     FRIEND_TEST(StatsLogProcessorTest, TestActivationOnBootMultipleActivations);
diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h
index 8587e1452543..7cac026c2421 100644
--- a/cmds/statsd/src/guardrail/StatsdStats.h
+++ b/cmds/statsd/src/guardrail/StatsdStats.h
@@ -660,6 +660,8 @@ private:
     FRIEND_TEST(StatsdStatsTest, TestAtomMetricsStats);
     FRIEND_TEST(StatsdStatsTest, TestActivationBroadcastGuardrailHit);
     FRIEND_TEST(StatsdStatsTest, TestAtomErrorStats);
+
+    FRIEND_TEST(StatsLogProcessorTest, InvalidConfigRemoved);
 };
 
 }  // namespace statsd
diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp
index 076f32752223..9a9702c34562 100644
--- a/cmds/statsd/tests/StatsLogProcessor_test.cpp
+++ b/cmds/statsd/tests/StatsLogProcessor_test.cpp
@@ -324,6 +324,41 @@ TEST(StatsLogProcessorTest, TestPullUidProviderSetOnConfigUpdate) {
     EXPECT_EQ(pullerManager->mPullUidProviders.find(key), pullerManager->mPullUidProviders.end());
 }
 
+TEST(StatsLogProcessorTest, InvalidConfigRemoved) {
+    // Setup simple config key corresponding to empty config.
+    StatsdStats::getInstance().reset();
+    sp<UidMap> m = new UidMap();
+    sp<StatsPullerManager> pullerManager = new StatsPullerManager();
+    m->updateMap(1, {1, 2}, {1, 2}, {String16("v1"), String16("v2")},
+                 {String16("p1"), String16("p2")}, {String16(""), String16("")});
+    sp<AlarmMonitor> anomalyAlarmMonitor;
+    sp<AlarmMonitor> subscriberAlarmMonitor;
+    StatsLogProcessor p(m, pullerManager, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
+                        [](const ConfigKey& key) { return true; },
+                        [](const int&, const vector<int64_t>&) {return true;});
+    ConfigKey key(3, 4);
+    StatsdConfig config = MakeConfig(true);
+    p.OnConfigUpdated(0, key, config);
+    EXPECT_EQ(1, p.mMetricsManagers.size());
+    EXPECT_NE(p.mMetricsManagers.find(key), p.mMetricsManagers.end());
+    // Cannot assert the size of mConfigStats since it is static and does not get cleared on reset.
+    EXPECT_NE(StatsdStats::getInstance().mConfigStats.end(),
+              StatsdStats::getInstance().mConfigStats.find(key));
+    EXPECT_EQ(0, StatsdStats::getInstance().mIceBox.size());
+
+    StatsdConfig invalidConfig = MakeConfig(true);
+    invalidConfig.clear_allowed_log_source();
+    p.OnConfigUpdated(0, key, invalidConfig);
+    EXPECT_EQ(0, p.mMetricsManagers.size());
+    // The current configs should not contain the invalid config.
+    EXPECT_EQ(StatsdStats::getInstance().mConfigStats.end(),
+              StatsdStats::getInstance().mConfigStats.find(key));
+    // Both "config" and "invalidConfig" should be in the icebox.
+    EXPECT_EQ(2, StatsdStats::getInstance().mIceBox.size());
+
+}
+
+
 TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead) {
     int uid = 1111;