Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove locks from StateManager to avoid race condition

Browse Source 
A race condition is introduced in ValueMetric due to StateManager's
locks. When StateManager processes a log event, it notifies all
listeners (metric producers) if a state has changed. In ValueMetric, a
state change eventually requires querying through a Statemanager
function that is held by the same lock as StateManager::onLogEvent.

To fix this, we can remove the locks in StateManager and use the locks
in StatsLogProcessor to protect StateManager objects. However, we have
to ensure that the path to StateManager function calls all originate
from StatsLogProcessor.

Test: bit statsd_test:*
Bug: 145838132
Change-Id: Ie5b747961ffa4cd5c78e86fa5f78472e29a972dd
This commit is contained in:
tsaichristine
2019-12-06 02:20:00 -08:00
parent f3d0c6f897
commit 7192e0a8e0
2 changed files with 9 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
cmds/statsd/src/state/StateManager.cpp
View File

@@ -29,18 +29,15 @@ StateManager& StateManager::getInstance() {
}
void StateManager::onLogEvent(const LogEvent& event) {
std::lock_guard<std::mutex> lock(mMutex);
if (mStateTrackers.find(event.GetTagId()) != mStateTrackers.end()) {
mStateTrackers[event.GetTagId()]->onLogEvent(event);
}
}
bool StateManager::registerListener(int32_t atomId, wp<StateListener> listener) {
std::lock_guard<std::mutex> lock(mMutex);
// Check if state tracker already exists
// Check if state tracker already exists.
if (mStateTrackers.find(atomId) == mStateTrackers.end()) {
// Create a new state tracker iff atom is a state atom
// Create a new state tracker iff atom is a state atom.
auto it = android::util::AtomsInfo::kStateAtomsFieldOptions.find(atomId);
if (it != android::util::AtomsInfo::kStateAtomsFieldOptions.end()) {
mStateTrackers[atomId] = new StateTracker(atomId, it->second);
@@ -79,8 +76,6 @@ void StateManager::unregisterListener(int32_t atomId, wp<StateListener> listener
bool StateManager::getStateValue(int32_t atomId, const HashableDimensionKey& key,
FieldValue* output) const {
std::lock_guard<std::mutex> lock(mMutex);
auto it = mStateTrackers.find(atomId);
if (it != mStateTrackers.end()) {
return it->second->getStateValue(key, output);

13
cmds/statsd/src/state/StateManager.h
View File

@@ -27,6 +27,10 @@ namespace android {
namespace os {
namespace statsd {
/**
* This class is NOT thread safe.
* It should only be used while StatsLogProcessor's lock is held.
*/
class StateManager : public virtual RefBase {
public:
StateManager(){};
@@ -56,13 +60,10 @@ public:
FieldValue* output) const;
inline int getStateTrackersCount() const {
std::lock_guard<std::mutex> lock(mMutex);
return mStateTrackers.size();
}
inline int getListenersCount(int32_t atomId) const {
std::lock_guard<std::mutex> lock(mMutex);
auto it = mStateTrackers.find(atomId);
if (it != mStateTrackers.end()) {
return it->second->getListenersCount();
@@ -71,10 +72,10 @@ public:
}
private:
mutable std::mutex mMutex;
mutable std::mutex mMutex;
// Maps state atom ids to StateTrackers
std::unordered_map<int32_t, sp<StateTracker>> mStateTrackers;
// Maps state atom ids to StateTrackers
std::unordered_map<int32_t, sp<StateTracker>> mStateTrackers;
};
} // namespace statsd