Fix StatsCompanionService sometimes can be null
Bug: 75970648
Test: will add cts test for puller alarms
Change-Id: I51b7d13f855d3c8ded8325d7cf0f614531eceea5
diff --git a/cmds/statsd/Android.mk b/cmds/statsd/Android.mk
index ca0611b..642567b 100644
--- a/cmds/statsd/Android.mk
+++ b/cmds/statsd/Android.mk
@@ -62,6 +62,7 @@
src/storage/StorageManager.cpp \
src/StatsLogProcessor.cpp \
src/StatsService.cpp \
+ src/statscompanion_util.cpp \
src/subscriber/IncidentdReporter.cpp \
src/subscriber/SubscriberReporter.cpp \
src/HashableDimensionKey.cpp \
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index b86646a..b03b4b4 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -49,33 +49,6 @@
constexpr const char* kPermissionDump = "android.permission.DUMP";
#define STATS_SERVICE_DIR "/data/misc/stats-service"
-/**
- * Watches for the death of the stats companion (system process).
- */
-class CompanionDeathRecipient : public IBinder::DeathRecipient {
-public:
- CompanionDeathRecipient(const sp<AlarmMonitor>& anomalyAlarmMonitor,
- const sp<AlarmMonitor>& periodicAlarmMonitor,
- const sp<StatsLogProcessor>& processor)
- : mAnomalyAlarmMonitor(anomalyAlarmMonitor),
- mPeriodicAlarmMonitor(periodicAlarmMonitor),
- mProcessor(processor) {}
- virtual void binderDied(const wp<IBinder>& who);
-
-private:
- sp<AlarmMonitor> mAnomalyAlarmMonitor;
- sp<AlarmMonitor> mPeriodicAlarmMonitor;
- sp<StatsLogProcessor> mProcessor;
-};
-
-void CompanionDeathRecipient::binderDied(const wp<IBinder>& who) {
- ALOGW("statscompanion service died");
- mProcessor->WriteDataToDisk();
- mAnomalyAlarmMonitor->setStatsCompanionService(nullptr);
- mPeriodicAlarmMonitor->setStatsCompanionService(nullptr);
- SubscriberReporter::getInstance().setStatsCompanionService(nullptr);
-}
-
StatsService::StatsService(const sp<Looper>& handlerLooper)
: mAnomalyAlarmMonitor(new AlarmMonitor(MIN_DIFF_TO_UPDATE_REGISTERED_ALARM_SECS,
[](const sp<IStatsCompanionService>& sc, int64_t timeMillis) {
@@ -791,21 +764,6 @@
}
}
-sp<IStatsCompanionService> StatsService::getStatsCompanionService() {
- sp<IStatsCompanionService> statsCompanion = nullptr;
- // Get statscompanion service from service manager
- const sp<IServiceManager> sm(defaultServiceManager());
- if (sm != nullptr) {
- const String16 name("statscompanion");
- statsCompanion = interface_cast<IStatsCompanionService>(sm->checkService(name));
- if (statsCompanion == nullptr) {
- ALOGW("statscompanion service unavailable!");
- return nullptr;
- }
- }
- return statsCompanion;
-}
-
Status StatsService::statsCompanionReady() {
VLOG("StatsService::statsCompanionReady was called");
@@ -821,9 +779,8 @@
"statscompanion unavailable despite it contacting statsd!");
}
VLOG("StatsService::statsCompanionReady linking to statsCompanion.");
- IInterface::asBinder(statsCompanion)
- ->linkToDeath(new CompanionDeathRecipient(
- mAnomalyAlarmMonitor, mPeriodicAlarmMonitor, mProcessor));
+ IInterface::asBinder(statsCompanion)->linkToDeath(this);
+ mStatsPullerManager.SetStatsCompanionService(statsCompanion);
mAnomalyAlarmMonitor->setStatsCompanionService(statsCompanion);
mPeriodicAlarmMonitor->setStatsCompanionService(statsCompanion);
SubscriberReporter::getInstance().setStatsCompanionService(statsCompanion);
@@ -969,6 +926,12 @@
void StatsService::binderDied(const wp <IBinder>& who) {
+ ALOGW("statscompanion service died");
+ mProcessor->WriteDataToDisk();
+ mAnomalyAlarmMonitor->setStatsCompanionService(nullptr);
+ mPeriodicAlarmMonitor->setStatsCompanionService(nullptr);
+ SubscriberReporter::getInstance().setStatsCompanionService(nullptr);
+ mStatsPullerManager.SetStatsCompanionService(nullptr);
}
} // namespace statsd
diff --git a/cmds/statsd/src/StatsService.h b/cmds/statsd/src/StatsService.h
index 0ec31ef..8d2fd33 100644
--- a/cmds/statsd/src/StatsService.h
+++ b/cmds/statsd/src/StatsService.h
@@ -23,6 +23,7 @@
#include "config/ConfigManager.h"
#include "external/StatsPullerManager.h"
#include "packages/UidMap.h"
+#include "statscompanion_util.h"
#include <android/os/BnStatsManager.h>
#include <android/os/IStatsCompanionService.h>
@@ -132,9 +133,6 @@
/** Inform statsCompanion that statsd is ready. */
virtual void sayHiToStatsCompanion();
- /** Fetches and returns the StatsCompanionService. */
- static sp<IStatsCompanionService> getStatsCompanionService();
-
/** IBinder::DeathRecipient */
virtual void binderDied(const wp<IBinder>& who) override;
diff --git a/cmds/statsd/src/external/StatsCompanionServicePuller.cpp b/cmds/statsd/src/external/StatsCompanionServicePuller.cpp
index bd859fd..d953f50 100644
--- a/cmds/statsd/src/external/StatsCompanionServicePuller.cpp
+++ b/cmds/statsd/src/external/StatsCompanionServicePuller.cpp
@@ -20,10 +20,9 @@
#include <android/os/IStatsCompanionService.h>
#include <binder/IPCThreadState.h>
#include <private/android_filesystem_config.h>
+#include "../stats_log_util.h"
+#include "../statscompanion_util.h"
#include "StatsCompanionServicePuller.h"
-#include "StatsService.h"
-#include "stats_log_util.h"
-#include "guardrail/StatsdStats.h"
using namespace android;
using namespace android::base;
@@ -44,11 +43,18 @@
StatsCompanionServicePuller::StatsCompanionServicePuller(int tagId) : StatsPuller(tagId) {
}
+void StatsCompanionServicePuller::SetStatsCompanionService(
+ sp<IStatsCompanionService> statsCompanionService) {
+ AutoMutex _l(mStatsCompanionServiceLock);
+ sp<IStatsCompanionService> tmpForLock = mStatsCompanionService;
+ mStatsCompanionService = statsCompanionService;
+}
+
bool StatsCompanionServicePuller::PullInternal(vector<shared_ptr<LogEvent> >* data) {
- sp<IStatsCompanionService> statsCompanion = StatsService::getStatsCompanionService();
- vector<StatsLogEventWrapper> returned_value;
- if (statsCompanion != NULL) {
- Status status = statsCompanion->pullData(mTagId, &returned_value);
+ sp<IStatsCompanionService> statsCompanionServiceCopy = mStatsCompanionService;
+ if (statsCompanionServiceCopy != nullptr) {
+ vector<StatsLogEventWrapper> returned_value;
+ Status status = statsCompanionServiceCopy->pullData(mTagId, &returned_value);
if (!status.isOk()) {
ALOGW("error pulling for %d", mTagId);
return false;
diff --git a/cmds/statsd/src/external/StatsCompanionServicePuller.h b/cmds/statsd/src/external/StatsCompanionServicePuller.h
index 4c91f31..0a49732 100644
--- a/cmds/statsd/src/external/StatsCompanionServicePuller.h
+++ b/cmds/statsd/src/external/StatsCompanionServicePuller.h
@@ -27,6 +27,12 @@
public:
StatsCompanionServicePuller(int tagId);
bool PullInternal(vector<std::shared_ptr<LogEvent> >* data) override;
+
+ void SetStatsCompanionService(sp<IStatsCompanionService> statsCompanionService) override;
+
+private:
+ Mutex mStatsCompanionServiceLock;
+ sp<IStatsCompanionService> mStatsCompanionService = nullptr;
};
} // namespace statsd
diff --git a/cmds/statsd/src/external/StatsPuller.h b/cmds/statsd/src/external/StatsPuller.h
index 82a8611..936c47e 100644
--- a/cmds/statsd/src/external/StatsPuller.h
+++ b/cmds/statsd/src/external/StatsPuller.h
@@ -16,9 +16,9 @@
#pragma once
-#include <android/os/StatsLogEventWrapper.h>
-#include <utils/String16.h>
+#include <android/os/IStatsCompanionService.h>
#include <utils/RefBase.h>
+#include <utils/String16.h>
#include <mutex>
#include <vector>
#include "packages/UidMap.h"
@@ -27,8 +27,6 @@
#include "logd/LogEvent.h"
#include "puller_util.h"
-using android::os::StatsLogEventWrapper;
-
namespace android {
namespace os {
namespace statsd {
@@ -49,7 +47,9 @@
static void SetUidMap(const sp<UidMap>& uidMap);
- protected:
+ virtual void SetStatsCompanionService(sp<IStatsCompanionService> statsCompanionService){};
+
+protected:
// The atom tag id this puller pulls
const int mTagId;
diff --git a/cmds/statsd/src/external/StatsPullerManager.h b/cmds/statsd/src/external/StatsPullerManager.h
index 0dee342..2717d5c 100644
--- a/cmds/statsd/src/external/StatsPullerManager.h
+++ b/cmds/statsd/src/external/StatsPullerManager.h
@@ -58,6 +58,10 @@
return mPullerManager.ForceClearPullerCache();
}
+ void SetStatsCompanionService(sp<IStatsCompanionService> statsCompanionService) {
+ mPullerManager.SetStatsCompanionService(statsCompanionService);
+ }
+
int ClearPullerCacheIfNecessary(long timestampSec) {
return mPullerManager.ClearPullerCacheIfNecessary(timestampSec);
}
diff --git a/cmds/statsd/src/external/StatsPullerManagerImpl.cpp b/cmds/statsd/src/external/StatsPullerManagerImpl.cpp
index fb0be73..dd6406b 100644
--- a/cmds/statsd/src/external/StatsPullerManagerImpl.cpp
+++ b/cmds/statsd/src/external/StatsPullerManagerImpl.cpp
@@ -21,15 +21,15 @@
#include <cutils/log.h>
#include <algorithm>
#include <climits>
+#include "../logd/LogEvent.h"
+#include "../stats_log_util.h"
+#include "../statscompanion_util.h"
#include "ResourceHealthManagerPuller.h"
#include "ResourceThermalManagerPuller.h"
#include "StatsCompanionServicePuller.h"
-#include "StatsPullerManagerImpl.h"
#include "StatsService.h"
#include "SubsystemSleepStatePuller.h"
-#include "logd/LogEvent.h"
#include "statslog.h"
-#include "stats_log_util.h"
#include <iostream>
@@ -123,7 +123,6 @@
StatsPullerManagerImpl::StatsPullerManagerImpl()
: mCurrentPullingInterval(LONG_MAX) {
- mStatsCompanionService = StatsService::getStatsCompanionService();
}
bool StatsPullerManagerImpl::Pull(int tagId, vector<shared_ptr<LogEvent>>* data) {
@@ -148,9 +147,35 @@
return kAllPullAtomInfo.find(tagId) != kAllPullAtomInfo.end();
}
+void StatsPullerManagerImpl::updateAlarmLocked() {
+ long currentTimeMs = getElapsedRealtimeMillis();
+ long nextAlarmTimeMs = currentTimeMs + mCurrentPullingInterval -
+ (currentTimeMs - mTimeBaseSec * 1000) % mCurrentPullingInterval;
+ sp<IStatsCompanionService> statsCompanionServiceCopy = mStatsCompanionService;
+ if (statsCompanionServiceCopy != nullptr) {
+ statsCompanionServiceCopy->setPullingAlarms(nextAlarmTimeMs, mCurrentPullingInterval);
+ } else {
+ VLOG("StatsCompanionService not available. Alarm not set.");
+ }
+ return;
+}
+
+void StatsPullerManagerImpl::SetStatsCompanionService(
+ sp<IStatsCompanionService> statsCompanionService) {
+ AutoMutex _l(mLock);
+ sp<IStatsCompanionService> tmpForLock = mStatsCompanionService;
+ mStatsCompanionService = statsCompanionService;
+ for (const auto& pulledAtom : kAllPullAtomInfo) {
+ pulledAtom.second.puller->SetStatsCompanionService(statsCompanionService);
+ }
+ if (mStatsCompanionService != nullptr) {
+ updateAlarmLocked();
+ }
+}
+
void StatsPullerManagerImpl::RegisterReceiver(int tagId, wp<PullDataReceiver> receiver,
long intervalMs) {
- AutoMutex _l(mReceiversLock);
+ AutoMutex _l(mLock);
auto& receivers = mReceivers[tagId];
for (auto it = receivers.begin(); it != receivers.end(); it++) {
if (it->receiver == receiver) {
@@ -175,20 +200,13 @@
if (roundedIntervalMs < mCurrentPullingInterval) {
VLOG("Updating pulling interval %ld", intervalMs);
mCurrentPullingInterval = roundedIntervalMs;
- long currentTimeMs = getElapsedRealtimeMillis();
- long nextAlarmTimeMs = currentTimeMs + mCurrentPullingInterval -
- (currentTimeMs - mTimeBaseSec * 1000) % mCurrentPullingInterval;
- if (mStatsCompanionService != nullptr) {
- mStatsCompanionService->setPullingAlarms(nextAlarmTimeMs, mCurrentPullingInterval);
- } else {
- VLOG("Failed to update pulling interval");
- }
+ updateAlarmLocked();
}
VLOG("Puller for tagId %d registered of %d", tagId, (int)receivers.size());
}
void StatsPullerManagerImpl::UnRegisterReceiver(int tagId, wp<PullDataReceiver> receiver) {
- AutoMutex _l(mReceiversLock);
+ AutoMutex _l(mLock);
if (mReceivers.find(tagId) == mReceivers.end()) {
VLOG("Unknown pull code or no receivers: %d", tagId);
return;
@@ -204,7 +222,7 @@
}
void StatsPullerManagerImpl::OnAlarmFired() {
- AutoMutex _l(mReceiversLock);
+ AutoMutex _l(mLock);
uint64_t currentTimeMs = getElapsedRealtimeMillis();
diff --git a/cmds/statsd/src/external/StatsPullerManagerImpl.h b/cmds/statsd/src/external/StatsPullerManagerImpl.h
index 76a4c14..682ad33 100644
--- a/cmds/statsd/src/external/StatsPullerManagerImpl.h
+++ b/cmds/statsd/src/external/StatsPullerManagerImpl.h
@@ -67,16 +67,15 @@
int ClearPullerCacheIfNecessary(long timestampSec);
+ void SetStatsCompanionService(sp<IStatsCompanionService> statsCompanionService);
+
const static std::map<int, PullAtomInfo> kAllPullAtomInfo;
private:
StatsPullerManagerImpl();
- // use this to update alarm
sp<IStatsCompanionService> mStatsCompanionService = nullptr;
- sp<IStatsCompanionService> get_stats_companion_service();
-
typedef struct {
// pull_interval_sec : last_pull_time_sec
std::pair<uint64_t, uint64_t> timeInfo;
@@ -86,7 +85,10 @@
// mapping from simple matcher tagId to receivers
std::map<int, std::list<ReceiverInfo>> mReceivers;
- Mutex mReceiversLock;
+ // locks for data receiver and StatsCompanionService changes
+ Mutex mLock;
+
+ void updateAlarmLocked();
long mCurrentPullingInterval;
diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index c6112fd..50eca05 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -277,14 +277,6 @@
config.event_metric_size() + config.value_metric_size();
allMetricProducers.reserve(allMetricsCount);
StatsPullerManager statsPullerManager;
- // Align all buckets to same instant in MIN_BUCKET_SIZE_SEC, so that avoid alarm
- // clock will not grow very aggressive. New metrics will be delayed up to
- // MIN_BUCKET_SIZE_SEC before starting.
- // Why not use timeBaseSec directly?
-// long currentTimeSec = time(nullptr);
-// uint64_t startTimeNs = (currentTimeSec - kMinBucketSizeSec -
-// (currentTimeSec - timeBaseSec) % kMinBucketSizeSec) *
-// NS_PER_SEC;
uint64_t startTimeNs = timeBaseSec * NS_PER_SEC;
diff --git a/cmds/statsd/src/stats_util.h b/cmds/statsd/src/stats_util.h
index e0206d1..5fcb161 100644
--- a/cmds/statsd/src/stats_util.h
+++ b/cmds/statsd/src/stats_util.h
@@ -28,9 +28,6 @@
const HashableDimensionKey DEFAULT_DIMENSION_KEY = HashableDimensionKey();
const MetricDimensionKey DEFAULT_METRIC_DIMENSION_KEY = MetricDimensionKey();
-// Minimum bucket size in seconds
-const long kMinBucketSizeSec = 5 * 60;
-
typedef std::map<int64_t, HashableDimensionKey> ConditionKey;
typedef std::unordered_map<MetricDimensionKey, int64_t> DimToValMap;
diff --git a/cmds/statsd/src/statscompanion_util.cpp b/cmds/statsd/src/statscompanion_util.cpp
new file mode 100644
index 0000000..d338827
--- /dev/null
+++ b/cmds/statsd/src/statscompanion_util.cpp
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define DEBUG false // STOPSHIP if true
+#include "Log.h"
+
+#include "statscompanion_util.h"
+
+namespace android {
+namespace os {
+namespace statsd {
+
+sp <IStatsCompanionService> getStatsCompanionService() {
+ sp<IStatsCompanionService> statsCompanion = nullptr;
+ // Get statscompanion service from service manager
+ static const sp <IServiceManager> sm(defaultServiceManager());
+ if (statsCompanion == nullptr) {
+ if (sm != nullptr) {
+ const String16 name("statscompanion");
+ statsCompanion = interface_cast<IStatsCompanionService>(sm->checkService(name));
+ if (statsCompanion == nullptr) {
+ ALOGW("statscompanion service unavailable!");
+ return nullptr;
+ }
+ }
+ }
+ return statsCompanion;
+}
+
+} // namespace statsd
+} // namespace os
+} // namespace android
diff --git a/cmds/statsd/src/statscompanion_util.h b/cmds/statsd/src/statscompanion_util.h
new file mode 100644
index 0000000..ff702f2
--- /dev/null
+++ b/cmds/statsd/src/statscompanion_util.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include "StatsLogProcessor.h"
+
+using namespace android;
+using namespace android::base;
+using namespace android::binder;
+using namespace android::os;
+using namespace std;
+
+namespace android {
+namespace os {
+namespace statsd {
+
+/** Fetches and returns the StatsCompanionService. */
+sp<IStatsCompanionService> getStatsCompanionService();
+
+} // namespace statsd
+} // namespace os
+} // namespace android