Merge cherrypicks of ['googleplex-android-review.googlesource.com/27324868'] into 24D1-release.
Change-Id: Ic667c93c65571cc6fb64a6f3fba832f2b59faa63
diff --git a/statsd/src/StatsService.cpp b/statsd/src/StatsService.cpp
index 86c2de8..afd5194 100644
--- a/statsd/src/StatsService.cpp
+++ b/statsd/src/StatsService.cpp
@@ -234,6 +234,7 @@
}
StatsService::~StatsService() {
+ onStatsdInitCompletedHandlerTermination();
if (mEventQueue != nullptr) {
stopReadingLogs();
mLogsReaderThread->join();
@@ -1062,6 +1063,7 @@
Status StatsService::informDeviceShutdown() {
ENFORCE_UID(AID_SYSTEM);
VLOG("StatsService::informDeviceShutdown");
+ onStatsdInitCompletedHandlerTermination();
int64_t elapsedRealtimeNs = getElapsedRealtimeNs();
int64_t wallClockNs = getWallClockNs();
mProcessor->WriteDataToDisk(DEVICE_SHUTDOWN, FAST, elapsedRealtimeNs, wallClockNs);
@@ -1115,7 +1117,14 @@
// This function is called from a dedicated thread without holding locks, so sleeping is ok.
// See MultiConditionTrigger::markComplete() executorThread for details
// For more details see http://b/277958338
- std::this_thread::sleep_for(std::chrono::seconds(mInitEventDelaySecs));
+
+ std::unique_lock<std::mutex> lk(mStatsdInitCompletedHandlerTerminationFlagMutex);
+ if (mStatsdInitCompletedHandlerTerminationFlag.wait_for(
+ lk, std::chrono::seconds(mInitEventDelaySecs),
+ [this] { return mStatsdInitCompletedHandlerTerminationRequested; })) {
+ VLOG("StatsService::onStatsdInitCompleted() Early termination is requested");
+ return;
+ }
}
mProcessor->onStatsdInitCompleted(getElapsedRealtimeNs());
@@ -1132,6 +1141,7 @@
void StatsService::Terminate() {
ALOGI("StatsService::Terminating");
+ onStatsdInitCompletedHandlerTermination();
if (mProcessor != nullptr) {
int64_t elapsedRealtimeNs = getElapsedRealtimeNs();
int64_t wallClockNs = getWallClockNs();
@@ -1142,6 +1152,14 @@
}
}
+void StatsService::onStatsdInitCompletedHandlerTermination() {
+ {
+ std::unique_lock<std::mutex> lk(mStatsdInitCompletedHandlerTerminationFlagMutex);
+ mStatsdInitCompletedHandlerTerminationRequested = true;
+ }
+ mStatsdInitCompletedHandlerTerminationFlag.notify_all();
+}
+
// Test only interface!!!
void StatsService::OnLogEvent(LogEvent* event) {
mProcessor->OnLogEvent(event);
@@ -1400,6 +1418,7 @@
void StatsService::statsCompanionServiceDiedImpl() {
ALOGW("statscompanion service died");
StatsdStats::getInstance().noteSystemServerRestart(getWallClockSec());
+ onStatsdInitCompletedHandlerTermination();
if (mProcessor != nullptr) {
ALOGW("Reset statsd upon system server restarts.");
int64_t systemServerRestartNs = getElapsedRealtimeNs();
diff --git a/statsd/src/StatsService.h b/statsd/src/StatsService.h
index 0a167dd..6a5d954 100644
--- a/statsd/src/StatsService.h
+++ b/statsd/src/StatsService.h
@@ -416,10 +416,15 @@
void onStatsdInitCompleted();
/*
- * This method is used to stop log reader thread.
+ * This method is used to stop log reader thread.
*/
void stopReadingLogs();
+ /*
+ * Notify async StatsdInitCompleted handler about termination event
+ */
+ void onStatsdInitCompletedHandlerTermination();
+
std::atomic<bool> mIsStopRequested = false;
/**
@@ -468,6 +473,14 @@
std::unique_ptr<std::thread> mLogsReaderThread;
+ std::condition_variable mStatsdInitCompletedHandlerTerminationFlag;
+ std::mutex mStatsdInitCompletedHandlerTerminationFlagMutex;
+ /**
+ * @brief Used to communicated early termination request to onStatsdInitCompleted Handler
+ * @see onStatsdInitCompleted
+ */
+ bool mStatsdInitCompletedHandlerTerminationRequested = false;
+
MultiConditionTrigger mBootCompleteTrigger;
static const inline string kBootCompleteTag = "BOOT_COMPLETE";
static const inline string kUidMapReceivedTag = "UID_MAP";
diff --git a/statsd/src/utils/MultiConditionTrigger.cpp b/statsd/src/utils/MultiConditionTrigger.cpp
index d9ad25b..e574969 100644
--- a/statsd/src/utils/MultiConditionTrigger.cpp
+++ b/statsd/src/utils/MultiConditionTrigger.cpp
@@ -14,11 +14,10 @@
* limitations under the License.
*/
#define STATSD_DEBUG false // STOPSHIP if true
+#include "Log.h"
#include "MultiConditionTrigger.h"
-#include <thread>
-
namespace android {
namespace os {
namespace statsd {
@@ -33,8 +32,7 @@
mTrigger(std::move(trigger)),
mCompleted(mRemainingConditionNames.empty()) {
if (mCompleted) {
- std::thread executorThread([this] { mTrigger(); });
- executorThread.detach();
+ startExecutorThread();
}
}
@@ -50,10 +48,21 @@
doTrigger = mCompleted;
}
if (doTrigger) {
- std::thread executorThread([this] { mTrigger(); });
- executorThread.detach();
+ startExecutorThread();
}
}
+
+void MultiConditionTrigger::startExecutorThread() {
+ mExecutorThread = std::make_unique<std::thread>([this] { mTrigger(); });
+}
+
+MultiConditionTrigger::~MultiConditionTrigger() {
+ if (mExecutorThread != nullptr && mExecutorThread->joinable()) {
+ VLOG("MultiConditionTrigger waiting on execution thread termination");
+ mExecutorThread->join();
+ }
+}
+
} // namespace statsd
} // namespace os
} // namespace android
diff --git a/statsd/src/utils/MultiConditionTrigger.h b/statsd/src/utils/MultiConditionTrigger.h
index 7a09119..773197d 100644
--- a/statsd/src/utils/MultiConditionTrigger.h
+++ b/statsd/src/utils/MultiConditionTrigger.h
@@ -19,6 +19,7 @@
#include <mutex>
#include <set>
+#include <thread>
namespace android {
namespace os {
@@ -27,8 +28,8 @@
/**
* This class provides a utility to wait for a set of named conditions to occur.
*
- * It will execute the trigger runnable in a detached thread once all conditions have been marked
- * true.
+ * It will execute the trigger runnable in a separate thread (which will be joined at instance
+ * destructor time) once all conditions have been marked true.
*/
class MultiConditionTrigger {
public:
@@ -37,16 +38,22 @@
MultiConditionTrigger(const MultiConditionTrigger&) = delete;
MultiConditionTrigger& operator=(const MultiConditionTrigger&) = delete;
+ ~MultiConditionTrigger();
// Mark a specific condition as true. If this condition has called markComplete already or if
// the event was not specified in the constructor, the function is a no-op.
void markComplete(const std::string& conditionName);
private:
+ void startExecutorThread();
+
mutable std::mutex mMutex;
std::set<std::string> mRemainingConditionNames;
std::function<void()> mTrigger;
bool mCompleted;
+ std::unique_ptr<std::thread> mExecutorThread;
+
+ FRIEND_TEST(MultiConditionTriggerTest, TestCountDownCalledBySameEventName);
};
} // namespace statsd
diff --git a/statsd/tests/utils/MultiConditionTrigger_test.cpp b/statsd/tests/utils/MultiConditionTrigger_test.cpp
index 32cecd3..b525f75 100644
--- a/statsd/tests/utils/MultiConditionTrigger_test.cpp
+++ b/statsd/tests/utils/MultiConditionTrigger_test.cpp
@@ -22,6 +22,8 @@
#include <thread>
#include <vector>
+#include "tests/statsd_test_util.h"
+
#ifdef __ANDROID__
using namespace std;
@@ -166,6 +168,125 @@
}
}
+namespace {
+
+class TriggerDependency {
+public:
+ TriggerDependency(mutex& lock, condition_variable& cv, bool& triggerCalled, int& triggerCount)
+ : mLock(lock), mCv(cv), mTriggerCalled(triggerCalled), mTriggerCount(triggerCount) {
+ }
+
+ void someMethod() {
+ lock_guard lg(mLock);
+ mTriggerCount++;
+ mTriggerCalled = true;
+ mCv.notify_all();
+ }
+
+private:
+ mutex& mLock;
+ condition_variable& mCv;
+ bool& mTriggerCalled;
+ int& mTriggerCount;
+};
+
+} // namespace
+
+TEST(MultiConditionTrigger, TestTriggerHasSleep) {
+ const string t1 = "t1";
+ set<string> conditionNames = {t1};
+
+ mutex lock;
+ condition_variable cv;
+ bool triggerCalled = false;
+ int triggerCount = 0;
+
+ {
+ TriggerDependency dependency(lock, cv, triggerCalled, triggerCount);
+ MultiConditionTrigger trigger(conditionNames, [&dependency] {
+ std::this_thread::sleep_for(std::chrono::milliseconds(50));
+ dependency.someMethod();
+ });
+ trigger.markComplete(t1);
+
+ // Here dependency instance will go out of scope and the thread within MultiConditionTrigger
+ // after delay will try to call method of already destroyed class instance
+ // with leading crash if trigger execution thread is detached in MultiConditionTrigger
+ // Instead since the MultiConditionTrigger destructor happens before TriggerDependency
+ // destructor, MultiConditionTrigger destructor is waiting on execution thread termination
+ // with thread::join
+ }
+ // At this moment the executor thread guaranteed terminated by MultiConditionTrigger destructor
+
+ // Ensure that the trigger fired.
+ {
+ unique_lock<mutex> unique_lk(lock);
+ cv.wait(unique_lk, [&triggerCalled] { return triggerCalled; });
+ EXPECT_TRUE(triggerCalled);
+ EXPECT_EQ(triggerCount, 1);
+ }
+}
+
+TEST(MultiConditionTrigger, TestTriggerHasSleepEarlyTermination) {
+ const string t1 = "t1";
+ set<string> conditionNames = {t1};
+
+ mutex lock;
+ condition_variable cv;
+ bool triggerCalled = false;
+ int triggerCount = 0;
+
+ std::condition_variable triggerTerminationFlag;
+ std::mutex triggerTerminationFlagMutex;
+ bool terminationRequested = false;
+
+ // used for error threshold tolerance due to wait_for() is involved
+ const int64_t errorThresholdMs = 25;
+ const int64_t triggerEarlyTerminationDelayMs = 100;
+ const int64_t triggerStartNs = getElapsedRealtimeNs();
+ {
+ TriggerDependency dependency(lock, cv, triggerCalled, triggerCount);
+ MultiConditionTrigger trigger(
+ conditionNames, [&dependency, &triggerTerminationFlag, &triggerTerminationFlagMutex,
+ &lock, &triggerCalled, &cv, &terminationRequested] {
+ std::unique_lock<std::mutex> lk(triggerTerminationFlagMutex);
+ if (triggerTerminationFlag.wait_for(
+ lk, std::chrono::seconds(1),
+ [&terminationRequested] { return terminationRequested; })) {
+ // triggerTerminationFlag was notified - early termination is requested
+ lock_guard lg(lock);
+ triggerCalled = true;
+ cv.notify_all();
+ return;
+ }
+ dependency.someMethod();
+ });
+ trigger.markComplete(t1);
+
+ // notify to terminate trigger executor thread after triggerEarlyTerminationDelayMs
+ std::this_thread::sleep_for(std::chrono::milliseconds(triggerEarlyTerminationDelayMs));
+ {
+ std::unique_lock<std::mutex> lk(triggerTerminationFlagMutex);
+ terminationRequested = true;
+ }
+ triggerTerminationFlag.notify_all();
+ }
+ // At this moment the executor thread guaranteed terminated by MultiConditionTrigger destructor
+
+ // check that test duration is closer to 100ms rather to 1s
+ const int64_t triggerEndNs = getElapsedRealtimeNs();
+ EXPECT_LE(NanoToMillis(triggerEndNs - triggerStartNs),
+ triggerEarlyTerminationDelayMs + errorThresholdMs);
+
+ // Ensure that the trigger fired but not the dependency.someMethod().
+ {
+ unique_lock<mutex> unique_lk(lock);
+ cv.wait(unique_lk, [&triggerCalled] { return triggerCalled; });
+ EXPECT_TRUE(triggerCalled);
+ EXPECT_EQ(triggerCount, 0);
+ }
+}
+
} // namespace statsd
} // namespace os
} // namespace android
