UM: Add an expiration timeout for async policy requests.
This enforces a timeout on AsyncPolicyRequest() calls. The timeout
applies to the overall period an async policy takes to be evaluated. If
a timeout fires, the update manager defers straight to evaluating the
default policy.
The actual expiration handling is done by the EvaluationContext,
incorporating it as part of its deferred reevaluation logic. This allows
us to not add (and manage) a separate main loop event for the expiration
timeout.
Unit tests were added to ensure that normal/timeout events are scheduled
and removed correctly, and that evaluation context expiration is
enforced.
BUG=chromium:384094
TEST=Unit tests.
Change-Id: Ia40e0ac3d8ab68eed043013cc930d699f3c3db93
Reviewed-on: https://chromium-review.googlesource.com/205895
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/update_manager_unittest.cc b/update_manager/update_manager_unittest.cc
index 4a15e93..7040a34 100644
--- a/update_manager/update_manager_unittest.cc
+++ b/update_manager/update_manager_unittest.cc
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <unistd.h>
+
#include <algorithm>
#include <string>
#include <utility>
@@ -71,12 +73,23 @@
// class extends the DefaultPolicy class to allow extensions of the Policy
// class without extending nor changing this test.
class FailingPolicy : public DefaultPolicy {
+ public:
+ explicit FailingPolicy(int* num_called_p) : num_called_p_(num_called_p) {}
+ FailingPolicy() : FailingPolicy(nullptr) {}
virtual EvalStatus UpdateCheckAllowed(EvaluationContext* ec, State* state,
string* error,
UpdateCheckParams* result) const {
+ if (num_called_p_)
+ (*num_called_p_)++;
*error = "FailingPolicy failed.";
return EvalStatus::kFailed;
}
+
+ protected:
+ virtual std::string PolicyName() const override { return "FailingPolicy"; }
+
+ private:
+ int* num_called_p_;
};
// The LazyPolicy always returns EvalStatus::kAskMeAgainLater.
@@ -86,6 +99,40 @@
UpdateCheckParams* result) const {
return EvalStatus::kAskMeAgainLater;
}
+
+ protected:
+ virtual std::string PolicyName() const override { return "LazyPolicy"; }
+};
+
+// A policy that sleeps and returns EvalStatus::kAskMeAgainlater. Will check
+// that time is greater than a given threshold (if non-zero). Increments a
+// counter every time it is being querie, if a pointer to it is provided.
+class DelayPolicy : public DefaultPolicy {
+ public:
+ DelayPolicy(int sleep_secs, base::Time time_threshold, int* num_called_p)
+ : sleep_secs_(sleep_secs), time_threshold_(time_threshold),
+ num_called_p_(num_called_p) {}
+ virtual EvalStatus UpdateCheckAllowed(EvaluationContext* ec, State* state,
+ string* error,
+ UpdateCheckParams* result) const {
+ if (num_called_p_)
+ (*num_called_p_)++;
+ sleep(sleep_secs_);
+ // We check for a time threshold to ensure that the policy has some
+ // non-constant dependency. The threshold is far enough in the future to
+ // ensure that it does not fire immediately.
+ if (time_threshold_ < base::Time::Max())
+ ec->IsTimeGreaterThan(time_threshold_);
+ return EvalStatus::kAskMeAgainLater;
+ }
+
+ protected:
+ virtual std::string PolicyName() const override { return "DelayPolicy"; }
+
+ private:
+ int sleep_secs_;
+ base::Time time_threshold_;
+ int* num_called_p_;
};
// AccumulateCallsCallback() adds to the passed |acc| accumulator vector pairs
@@ -154,11 +201,68 @@
Callback<void(EvalStatus, const UpdateCheckParams& result)> callback =
Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
- umut_->AsyncPolicyRequest(callback, &Policy::UpdateCheckAllowed);
+ umut_->AsyncPolicyRequest(callback, base::TimeDelta::FromSeconds(5),
+ &Policy::UpdateCheckAllowed);
// The callback should wait until we run the main loop for it to be executed.
EXPECT_EQ(0, calls.size());
chromeos_update_engine::RunGMainLoopMaxIterations(100);
EXPECT_EQ(1, calls.size());
}
+TEST_F(UmUpdateManagerTest, AsyncPolicyRequestDoesNotTimeOut) {
+ // Set up an async policy call to return immediately, then wait a little and
+ // ensure that the timeout event does not fire.
+ int num_called = 0;
+ umut_->set_policy(new FailingPolicy(&num_called));
+
+ vector<pair<EvalStatus, UpdateCheckParams>> calls;
+ Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
+ Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
+
+ umut_->AsyncPolicyRequest(callback, base::TimeDelta::FromSeconds(1),
+ &Policy::UpdateCheckAllowed);
+ // Run the main loop, ensure that policy was attempted once before deferring
+ // to the default.
+ chromeos_update_engine::RunGMainLoopMaxIterations(100);
+ EXPECT_EQ(1, num_called);
+ ASSERT_EQ(1, calls.size());
+ EXPECT_EQ(EvalStatus::kSucceeded, calls[0].first);
+ // Wait for the timeout to expire, run the main loop again, ensure that
+ // nothing happened.
+ sleep(2);
+ chromeos_update_engine::RunGMainLoopMaxIterations(10);
+ EXPECT_EQ(1, num_called);
+ EXPECT_EQ(1, calls.size());
+}
+
+TEST_F(UmUpdateManagerTest, AsyncPolicyRequestTimesOut) {
+ // Set up an async policy call to exceed its overall execution time, and make
+ // sure that it is aborted. Also ensure that the async call is not reattempted
+ // after the timeout fires by waiting pas the time threshold for reevaluation.
+ int num_called = 0;
+ umut_->set_policy(new DelayPolicy(
+ 2, fake_clock_.GetWallclockTime() + base::TimeDelta::FromSeconds(3),
+ &num_called));
+
+ vector<pair<EvalStatus, UpdateCheckParams>> calls;
+ Callback<void(EvalStatus, const UpdateCheckParams&)> callback =
+ Bind(AccumulateCallsCallback<UpdateCheckParams>, &calls);
+
+ umut_->AsyncPolicyRequest(callback, base::TimeDelta::FromSeconds(1),
+ &Policy::UpdateCheckAllowed);
+ // Run the main loop, ensure that policy was attempted once but the callback
+ // was not invoked.
+ chromeos_update_engine::RunGMainLoopMaxIterations(100);
+ EXPECT_EQ(1, num_called);
+ EXPECT_EQ(0, calls.size());
+ // Wait for the time threshold to be satisfied, run the main loop again,
+ // ensure that reevaluation was not attempted but the callback invoked.
+ sleep(2);
+ chromeos_update_engine::RunGMainLoopMaxIterations(10);
+ EXPECT_EQ(1, num_called);
+ ASSERT_EQ(1, calls.size());
+ EXPECT_EQ(EvalStatus::kSucceeded, calls[0].first);
+ EXPECT_EQ(true, calls[0].second.updates_enabled);
+}
+
} // namespace chromeos_update_manager