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/evaluation_context_unittest.cc b/update_manager/evaluation_context_unittest.cc
index 52755fc..9835648 100644
--- a/update_manager/evaluation_context_unittest.cc
+++ b/update_manager/evaluation_context_unittest.cc
@@ -238,7 +238,7 @@
}
// Test that callback is called when evaluation context expires, and that it
-// cannot be used again.
+// cannot be used again unless the expiration deadline is reset.
TEST_F(UmEvaluationContextTest, RunOnValueChangeOrTimeoutExpires) {
fake_async_var_.reset(new string("Async value"));
eval_ctx_->GetValue(&fake_async_var_);
@@ -253,6 +253,10 @@
// Ensure that we cannot reschedule an evaluation.
EXPECT_FALSE(eval_ctx_->RunOnValueChangeOrTimeout(Bind(&DoNothing)));
+
+ // Ensure that we can reschedule an evaluation after resetting expiration.
+ eval_ctx_->ResetExpiration();
+ EXPECT_TRUE(eval_ctx_->RunOnValueChangeOrTimeout(Bind(&DoNothing)));
}
// Test that we clear the events when destroying the EvaluationContext.