PolicyManager: Call all the ValueChanged() on the same main loop call.
This patch calls the ValueChanged() method on all the observers from
the same main loop calling, fixing a possible free after use if the
observer gets removed and deleted between the NotifyValueChanged()
call and the ValueChanged() calls.
BUG=chromium:355132
TEST=Unittest added.
Change-Id: I6365c3b86fe0b85502bfa81bbd18a86c55caa009
Reviewed-on: https://chromium-review.googlesource.com/192526
Reviewed-by: Alex Deymo <[email protected]>
Commit-Queue: Alex Deymo <[email protected]>
Tested-by: Alex Deymo <[email protected]>
diff --git a/policy_manager/variable.h b/policy_manager/variable.h
index 22cdd17..ab19f72 100644
--- a/policy_manager/variable.h
+++ b/policy_manager/variable.h
@@ -102,10 +102,8 @@
// Calls ValueChanged on all the observers.
void NotifyValueChanged() {
- for (auto& observer : observer_list_) {
- RunFromMainLoop(base::Bind(&BaseVariable::ObserverInterface::ValueChanged,
- base::Unretained(observer), this));
- }
+ RunFromMainLoop(base::Bind(&BaseVariable::OnValueChangedNotification,
+ base::Unretained(this)));
}
private:
@@ -113,6 +111,7 @@
friend class PmBaseVariableTest;
FRIEND_TEST(PmBaseVariableTest, RepeatedObserverTest);
FRIEND_TEST(PmBaseVariableTest, NotifyValueChangedTest);
+ FRIEND_TEST(PmBaseVariableTest, NotifyValueRemovesObserversTest);
BaseVariable(const std::string& name, VariableMode mode,
base::TimeDelta poll_interval)
@@ -120,6 +119,22 @@
poll_interval_(mode == kVariableModePoll ?
poll_interval : base::TimeDelta()) {}
+ void OnValueChangedNotification() {
+ // A ValueChanged() method can change the list of observers, for example
+ // removing itself and invalidating the iterator, so we create a snapshot
+ // of the observers first. Also, to support the case when *another* observer
+ // is removed, we check for them.
+ std::list<BaseVariable::ObserverInterface*> observer_list_copy(
+ observer_list_);
+
+ for (auto& observer : observer_list_copy) {
+ if (std::find(observer_list_.begin(), observer_list_.end(), observer) !=
+ observer_list_.end()) {
+ observer->ValueChanged(this);
+ }
+ }
+ }
+
// The default PollInterval in minutes.
static constexpr int kDefaultPollMinutes = 5;
