UM: Fix async callback handling.
A bug in EvaluationContext::OnValueChangedOrPollTimeout() causes
scheduling of a reevaluation to fail. We fix it by copying the callback
pointer to a local variable and clearing it prior to invoking the
callback.
This also adds a check in the evaluation context unit tests code to
ensure that the EC under test is actually being destroyed; this is
useful to ensure that unit tests are not leaking EC references, e.g. by
form of pending main loop events that are holding EC handles in
closures.
BUG=chromium:391037
TEST=New unit test fails before fix, succeeds after it.
Change-Id: I63486b80525ec19d0cd399c52eea39d991a4ff53
Reviewed-on: https://chromium-review.googlesource.com/206538
Tested-by: Gilad Arnold <[email protected]>
Reviewed-by: Alex Deymo <[email protected]>
Commit-Queue: Gilad Arnold <[email protected]>
diff --git a/update_manager/evaluation_context.cc b/update_manager/evaluation_context.cc
index a24bf23..da69ced 100644
--- a/update_manager/evaluation_context.cc
+++ b/update_manager/evaluation_context.cc
@@ -59,10 +59,10 @@
void EvaluationContext::OnValueChangedOrPollTimeout() {
RemoveObserversAndTimeout();
- if (value_changed_callback_.get() != NULL) {
- value_changed_callback_->Run();
- value_changed_callback_.reset();
- }
+ // Copy the callback handle locally, allowing the callback code to reset it.
+ scoped_ptr<Closure> callback(value_changed_callback_.release());
+ if (callback.get() != NULL)
+ callback->Run();
}
bool EvaluationContext::IsTimeGreaterThan(base::Time timestamp) {
@@ -133,6 +133,9 @@
if (!waiting_for_value_change && !reeval_timeout_set)
return false;
if (reeval_timeout_set) {
+ DLOG(INFO)
+ << "Waiting for poll timeout in "
+ << chromeos_update_engine::utils::FormatTimeDelta(reeval_timeout);
poll_timeout_event_ = RunFromMainLoopAfterTimeout(
base::Bind(&EvaluationContext::OnPollTimeout,
weak_ptr_factory_.GetWeakPtr()),
