Merge "Fix StatsCompanionService sometimes can be null" into pi-dev

cmds/statsd/Android.mk

@@ -62,6 +62,7 @@ statsd_common_src := \
     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 \

cmds/statsd/src/StatsService.cpp

@@ -49,33 +49,6 @@ namespace statsd {
 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 @@ void StatsService::sayHiToStatsCompanion() {
     }
 }
 
-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 @@ Status StatsService::statsCompanionReady() {
                 "statscompanion unavailable despite it contacting statsd!");
     }
     VLOG("StatsService::statsCompanionReady linking to statsCompanion.");
-    IInterface::asBinder(statsCompanion)
+    IInterface::asBinder(statsCompanion)->linkToDeath(this);
-        ->linkToDeath(new CompanionDeathRecipient(
+    mStatsPullerManager.SetStatsCompanionService(statsCompanion);
-            mAnomalyAlarmMonitor, mPeriodicAlarmMonitor, mProcessor));
     mAnomalyAlarmMonitor->setStatsCompanionService(statsCompanion);
     mPeriodicAlarmMonitor->setStatsCompanionService(statsCompanion);
     SubscriberReporter::getInstance().setStatsCompanionService(statsCompanion);
@@ -969,6 +926,12 @@ Status StatsService::unsetBroadcastSubscriber(int64_t configId,
 
 
 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

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 @@ public:
     /** 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;
 

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 @@ const int kLogMsgHeaderSize = 28;
 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();
+    sp<IStatsCompanionService> statsCompanionServiceCopy = mStatsCompanionService;
-    vector<StatsLogEventWrapper> returned_value;
+    if (statsCompanionServiceCopy != nullptr) {
-    if (statsCompanion != NULL) {
+        vector<StatsLogEventWrapper> returned_value;
-        Status status = statsCompanion->pullData(mTagId, &returned_value);
+        Status status = statsCompanionServiceCopy->pullData(mTagId, &returned_value);
         if (!status.isOk()) {
             ALOGW("error pulling for %d", mTagId);
             return false;

cmds/statsd/src/external/StatsCompanionServicePuller.h

@@ -27,6 +27,12 @@ class StatsCompanionServicePuller : public StatsPuller {
 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

cmds/statsd/src/external/StatsPuller.h

@@ -16,9 +16,9 @@
 
 #pragma once
 
-#include <android/os/StatsLogEventWrapper.h>
+#include <android/os/IStatsCompanionService.h>
-#include <utils/String16.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 @@ public:
 
     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;
 

cmds/statsd/src/external/StatsPullerManager.h

@@ -58,6 +58,10 @@ class StatsPullerManager {
         return mPullerManager.ForceClearPullerCache();
     }
 
+    void SetStatsCompanionService(sp<IStatsCompanionService> statsCompanionService) {
+        mPullerManager.SetStatsCompanionService(statsCompanionService);
+    }
+
     int ClearPullerCacheIfNecessary(long timestampSec) {
         return mPullerManager.ClearPullerCacheIfNecessary(timestampSec);
     }

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 @@ const std::map<int, PullAtomInfo> StatsPullerManagerImpl::kAllPullAtomInfo = {
 
 StatsPullerManagerImpl::StatsPullerManagerImpl()
     : mCurrentPullingInterval(LONG_MAX) {
-    mStatsCompanionService = StatsService::getStatsCompanionService();
 }
 
 bool StatsPullerManagerImpl::Pull(int tagId, vector<shared_ptr<LogEvent>>* data) {
@@ -148,9 +147,35 @@ bool StatsPullerManagerImpl::PullerForMatcherExists(int tagId) const {
     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 @@ void StatsPullerManagerImpl::RegisterReceiver(int tagId, wp<PullDataReceiver> re
     if (roundedIntervalMs < mCurrentPullingInterval) {
         VLOG("Updating pulling interval %ld", intervalMs);
         mCurrentPullingInterval = roundedIntervalMs;
-        long currentTimeMs = getElapsedRealtimeMillis();
+        updateAlarmLocked();
-        long nextAlarmTimeMs = currentTimeMs + mCurrentPullingInterval -
-            (currentTimeMs - mTimeBaseSec * 1000) % mCurrentPullingInterval;
-        if (mStatsCompanionService != nullptr) {
-            mStatsCompanionService->setPullingAlarms(nextAlarmTimeMs, mCurrentPullingInterval);
-        } else {
-            VLOG("Failed to update pulling interval");
-        }
     }
     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::UnRegisterReceiver(int tagId, wp<PullDataReceiver>
 }
 
 void StatsPullerManagerImpl::OnAlarmFired() {
-    AutoMutex _l(mReceiversLock);
+    AutoMutex _l(mLock);
 
     uint64_t currentTimeMs = getElapsedRealtimeMillis();
 

cmds/statsd/src/external/StatsPullerManagerImpl.h

@@ -67,16 +67,15 @@ public:
 
     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 @@ public:
     // 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;
 

cmds/statsd/src/metrics/metrics_manager_util.cpp

@@ -277,14 +277,6 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const long ti
                                 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;
 

cmds/statsd/src/stats_util.h

@@ -28,9 +28,6 @@ namespace statsd {
 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;

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

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