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 <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/policy_manager/variable_unittest.cc b/policy_manager/variable_unittest.cc
index 81e1e59..f68f7d3 100644
--- a/policy_manager/variable_unittest.cc
+++ b/policy_manager/variable_unittest.cc
@@ -106,6 +106,53 @@
// Check that all the observers are called.
EXPECT_EQ(2, observer1.calls_.size());
EXPECT_EQ(1, observer2.calls_.size());
+
+ var.RemoveObserver(&observer1);
+ var.RemoveObserver(&observer2);
+}
+
+class BaseVariableObserverRemover : public BaseVariable::ObserverInterface {
+ public:
+ BaseVariableObserverRemover() : calls_(0) {}
+
+ void ValueChanged(BaseVariable* variable) override {
+ for (auto& observer : remove_observers_) {
+ variable->RemoveObserver(observer);
+ }
+ calls_++;
+ }
+
+ void OnCallRemoveObserver(BaseVariable::ObserverInterface* observer) {
+ remove_observers_.push_back(observer);
+ }
+
+ int get_calls() { return calls_; }
+
+ private:
+ vector<BaseVariable::ObserverInterface*> remove_observers_;
+ int calls_;
+};
+
+// Tests that we can remove an observer from a Variable on the ValueChanged()
+// call to that observer.
+TEST(PmBaseVariableTest, NotifyValueRemovesObserversTest) {
+ DefaultVariable<int> var("var", kVariableModeAsync);
+ BaseVariableObserverRemover observer1;
+ BaseVariableObserverRemover observer2;
+
+ var.AddObserver(&observer1);
+ var.AddObserver(&observer2);
+
+ // Make each observer remove both observers on ValueChanged.
+ observer1.OnCallRemoveObserver(&observer1);
+ observer1.OnCallRemoveObserver(&observer2);
+ observer2.OnCallRemoveObserver(&observer1);
+ observer2.OnCallRemoveObserver(&observer2);
+
+ var.NotifyValueChanged();
+ RunGMainLoopMaxIterations(100);
+
+ EXPECT_EQ(1, observer1.get_calls() + observer2.get_calls());
}
} // namespace chromeos_policy_manager