update_engine: UM: Async request expiration handled differently.
As discussed on the tracker issue, we're changing the way an async
policy request expiration timeout is being handled: instead of failing
the policy request entirely, this now only causes the UpdateManager to
dump the evaluation context, reset the expiration deadline and
reevaluate the policy (which is necessary in order for evaluation time
and corresponding timeouts to be recomputed). This is aimed to ensure
that policies are allowed to block for arbitrarily long periods, while
still emitting useful information to the log (which will help diagnose
if this is due to an implementation error).
Since the expiration timeout no longer returns control to the caller, we
remove it from the AsyncPolicyRequest() API. Instead, we use a single
timeout value, which is set during the UpdateManager construction and
used for all policy calls. By default, the update engine sets it to 12
hours; for testing and debugging purposes, a smaller value is used.
This CL also forbids the default (fallback) policy from blocking,
forcing a failure instead; a situation like that makes no sense anyway,
and may lead to inconsistent return values leaking to the caller.
BUG=chromium:401687
TEST=Unit tests.
Change-Id: I0bf60875bb7f524c99ed72dac61720633ab2061b
Reviewed-on: https://chromium-review.googlesource.com/211647
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/update_manager.h b/update_manager/update_manager.h
index 0b04c07..2cd21d4 100644
--- a/update_manager/update_manager.h
+++ b/update_manager/update_manager.h
@@ -25,7 +25,8 @@
// Creates the UpdateManager instance, assuming ownership on the provided
// |state|.
UpdateManager(chromeos_update_engine::ClockInterface* clock,
- base::TimeDelta evaluation_timeout, State* state);
+ base::TimeDelta evaluation_timeout,
+ base::TimeDelta expiration_timeout, State* state);
virtual ~UpdateManager() {}
@@ -53,17 +54,15 @@
// Evaluates the given |policy_method| policy with the provided |args|
// arguments and calls the |callback| callback with the result when done.
- // Evaluation is not allowed to exceed |request_timeout|.
//
// If the policy implementation should block, returning a
// EvalStatus::kAskMeAgainLater status the Update Manager will re-evaluate the
// policy until another status is returned. If the policy implementation based
// its return value solely on const variables, the callback will be called
- // with the EvalStatus::kAskMeAgainLater status.
+ // with the EvalStatus::kAskMeAgainLater status (which indicates an error).
template<typename R, typename... ActualArgs, typename... ExpectedArgs>
void AsyncPolicyRequest(
base::Callback<void(EvalStatus, const R& result)> callback,
- base::TimeDelta request_timeout,
EvalStatus (Policy::*policy_method)(EvaluationContext*, State*,
std::string*, R*,
ExpectedArgs...) const,
@@ -83,7 +82,7 @@
FRIEND_TEST(UmUpdateManagerTest, PolicyRequestCallsDefaultOnError);
FRIEND_TEST(UmUpdateManagerTest, PolicyRequestDoesntBlockDeathTest);
FRIEND_TEST(UmUpdateManagerTest, AsyncPolicyRequestDelaysEvaluation);
- FRIEND_TEST(UmUpdateManagerTest, AsyncPolicyRequestDoesNotTimeOut);
+ FRIEND_TEST(UmUpdateManagerTest, AsyncPolicyRequestTimeoutDoesNotFire);
FRIEND_TEST(UmUpdateManagerTest, AsyncPolicyRequestTimesOut);
// EvaluatePolicy() evaluates the passed |policy_method| method on the current
@@ -129,6 +128,9 @@
// Timeout for a policy evaluation.
const base::TimeDelta evaluation_timeout_;
+ // Timeout for expiration of the evaluation context, used for async requests.
+ const base::TimeDelta expiration_timeout_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateManager);
};