Skip to content

GitLab

T
tangled
Commit 917821a0 authored by eugenis's avatar eugenis Committed by Commit bot
Browse files
Options

Revert of Add SyncScheduler for scheduling CryptAuth enrollments and syncing... 

Revert of Add SyncScheduler for scheduling CryptAuth enrollments and syncing devices. (patchset #3 id:80001 of https://codereview.chromium.org/1147563002/)

Reason for revert:
MSan and Valgrind failures (use-of-uninitialized memory).

Original issue's description:
> Add SyncScheduler for scheduling CryptAuth enrollments and syncing devices.
>
> BUG=420315
> TEST=unit test
>
> Committed: https://crrev.com/27e044cfc5c2a43b6c575508aca64763bfa2deba
> Cr-Commit-Position: refs/heads/master@{#330713}

TBR=isherman@chromium.org,tengs@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=420315

Review URL: https://codereview.chromium.org/1146193002

Cr-Commit-Position: refs/heads/master@{#330780}
parent cdc1058c
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 706 deletions
components/components_tests.gyp
View file @ 917821a0
......@@ -439,7 +439,6 @@
'proximity_auth/cryptauth/cryptauth_client_impl_unittest.cc',
'proximity_auth/cryptauth/cryptauth_enroller_impl_unittest.cc',
'proximity_auth/cryptauth/fake_secure_message_delegate_unittest.cc',
'proximity_auth/cryptauth/sync_scheduler_impl_unittest.cc',
'proximity_auth/logging/logging_unittest.cc',
'proximity_auth/proximity_auth_system_unittest.cc',
'proximity_auth/remote_status_update_unittest.cc',
......
components/proximity_auth.gypi
View file @ 917821a0
......@@ -123,10 +123,6 @@
"proximity_auth/cryptauth/cryptauth_enrollment_utils.h",
"proximity_auth/cryptauth/secure_message_delegate.cc",
"proximity_auth/cryptauth/secure_message_delegate.h",
"proximity_auth/cryptauth/sync_scheduler.cc",
"proximity_auth/cryptauth/sync_scheduler.h",
"proximity_auth/cryptauth/sync_scheduler_impl.cc",
"proximity_auth/cryptauth/sync_scheduler_impl.h",
],
'export_dependent_settings': [
'cryptauth_proto',
......
components/proximity_auth/cryptauth/BUILD.gn
View file @ 917821a0
......@@ -21,10 +21,6 @@ source_set("cryptauth") {
"cryptauth_enrollment_utils.h",
"secure_message_delegate.cc",
"secure_message_delegate.h",
"sync_scheduler.cc",
"sync_scheduler.h",
"sync_scheduler_impl.cc",
"sync_scheduler_impl.h",
]
deps = [
......@@ -69,7 +65,6 @@ source_set("unit_tests") {
"cryptauth_client_impl_unittest.cc",
"cryptauth_enroller_impl_unittest.cc",
"fake_secure_message_delegate_unittest.cc",
"sync_scheduler_impl_unittest.cc",
]
deps = [
......
components/proximity_auth/cryptauth/sync_scheduler.cc deleted 100644 → 0
View file @ cdc1058c
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/proximity_auth/cryptauth/sync_scheduler.h"
#include "components/proximity_auth/logging/logging.h"
namespace proximity_auth {
SyncScheduler::SyncRequest::SyncRequest(
base::WeakPtr<SyncScheduler> sync_scheduler)
: sync_scheduler_(sync_scheduler), completed_(false) {
}
SyncScheduler::SyncRequest::~SyncRequest() {
if (!completed_)
PA_LOG(ERROR) << "SyncRequest destroyed but Complete() was never called";
}
void SyncScheduler::SyncRequest::OnDidComplete(bool success) {
if (sync_scheduler_) {
sync_scheduler_->OnSyncCompleted(success);
sync_scheduler_.reset();
completed_ = true;
} else {
PA_LOG(ERROR) << "SyncRequest completed, but SyncScheduler destroyed.";
}
}
} // proximity_auth
components/proximity_auth/cryptauth/sync_scheduler.h deleted 100644 → 0
View file @ cdc1058c
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_SYNC_SCHEDULER_H
#define COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_SYNC_SCHEDULER_H
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
namespace proximity_auth {
// Interface for scheduling the next CryptAuth sync (e.g. enrollment or device
// sync). The scheduler has two different strategies affecting when to perform
// the next operation:
// PERIODIC_REFRESH: The last sync was made successfully, so we can wait a
// relatively long time before making another sync.
// AGGRESSIVE_RECOVERY: The last sync failed, so we try more aggressively to
// make enrollment attempts with subsequent backoff for repeated
// failures.
// A random jitter is applied to each sync period to smooth qps to the server.
class SyncScheduler {
public:
// The sync strategies mentioned in the class comments.
enum class Strategy { PERIODIC_REFRESH, AGGRESSIVE_RECOVERY };
// The states that the scheduler can be in.
enum class SyncState { NOT_STARTED, WAITING_FOR_REFRESH, SYNC_IN_PROGRESS };
// An instance is passed to the delegate when the scheduler fires for each
// sync attempt. The delegate should call |Complete()| when the sync succeeds
// or fails to resume the scheduler.
class SyncRequest {
public:
explicit SyncRequest(base::WeakPtr<SyncScheduler> sync_scheduler);
~SyncRequest();
void OnDidComplete(bool success);
protected:
// The parent scheduler that dispatched this request.
base::WeakPtr<SyncScheduler> sync_scheduler_;
// True if |OnDidComplete()| has been called.
bool completed_;
DISALLOW_COPY_AND_ASSIGN(SyncRequest);
};
// Handles the actual sync operation.
class Delegate {
public:
virtual ~Delegate() {}
// Called when the scheduler fires and requests a sync attempt. The delegate
// should call sync_request->Complete() when the request finishes.
virtual void OnSyncRequested(scoped_ptr<SyncRequest> sync_request) = 0;
};
virtual ~SyncScheduler() {}
// Starts the scheduler with an aggressive recovery strategy if
// |strategy| is true; otherwise, it will be started with
// periodic refresh.
//
// |elapsed_time_since_last_sync| is the time since the last successful sync,
// so we can determine the duration of the first sync period. For example, the
// scheduler will immediately issue a sync request if the elapsed time is
// greater than the refresh period.
virtual void Start(const base::TimeDelta& elapsed_time_since_last_sync,
Strategy strategy) = 0;
// Cancels the current scheduled sync, and forces a sync immediately. Note
// that if this sync fails, the scheduler will adopt the AGGRESSIVE_RECOVERY
// strategy.
virtual void ForceSync() = 0;
// Returns the time until the next scheduled sync operation. If no sync is
// scheduled, a TimeDelta of zero will be returned.
virtual base::TimeDelta GetTimeToNextSync() const = 0;
// Returns the current sync strategy.
virtual Strategy GetStrategy() const = 0;
// Returns the current state of the scheduler.
virtual SyncState GetSyncState() const = 0;
protected:
// Called by SyncRequest instances when the sync completes.
virtual void OnSyncCompleted(bool success) = 0;
};
} // namespace proximity_auth
#endif // COMPONENTS_PROXIMITY_CRYPTAUTH_SYNC_SCHEDULER_H
components/proximity_auth/cryptauth/sync_scheduler_impl.cc deleted 100644 → 0
View file @ cdc1058c
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/proximity_auth/cryptauth/sync_scheduler_impl.h"
#include <algorithm>
#include <cmath>
#include <limits>
#include "base/bind.h"
#include "base/numerics/safe_conversions.h"
#include "base/rand_util.h"
#include "base/strings/stringprintf.h"
#include "components/proximity_auth/logging/logging.h"
namespace proximity_auth {
namespace {
// Returns a human readable string given a |time_delta|.
std::string TimeDeltaToString(const base::TimeDelta& time_delta) {
if (time_delta.InDays() > 0)
return base::StringPrintf("%d days", time_delta.InDays());
if (time_delta.InHours() > 0)
return base::StringPrintf("%d hours", time_delta.InHours());
if (time_delta.InMinutes() > 0)
return base::StringPrintf("%d minutes", time_delta.InMinutes());
return base::StringPrintf("%d seconds",
base::saturated_cast<int>(time_delta.InSeconds()));
}
} // namespace
SyncSchedulerImpl::SyncSchedulerImpl(Delegate* delegate,
base::TimeDelta refresh_period,
base::TimeDelta base_recovery_period,
double max_jitter_ratio,
const std::string& scheduler_name)
: delegate_(delegate),
refresh_period_(refresh_period),
base_recovery_period_(base_recovery_period),
max_jitter_ratio_(max_jitter_ratio),
scheduler_name_(scheduler_name),
strategy_(Strategy::PERIODIC_REFRESH),
sync_state_(SyncState::NOT_STARTED),
weak_ptr_factory_(this) {
}
SyncSchedulerImpl::~SyncSchedulerImpl() {
}
void SyncSchedulerImpl::Start(
const base::TimeDelta& elapsed_time_since_last_sync,
Strategy strategy) {
strategy_ = strategy;
sync_state_ = SyncState::WAITING_FOR_REFRESH;
// We reset the failure backoff when the scheduler is started again, as the
// configuration that caused the previous attempts to fail most likely won't
// be present after a restart.
if (strategy_ == Strategy::AGGRESSIVE_RECOVERY)
failure_count_ = 1;
// To take into account the time waited when the system is powered off, we
// subtract the time elapsed with a normal sync period to the initial time
// to wait.
base::TimeDelta sync_delta =
GetJitteredPeriod() - elapsed_time_since_last_sync;
if (sync_delta < base::TimeDelta::FromSeconds(0))
sync_delta = base::TimeDelta::FromSeconds(0);
ScheduleNextSync(sync_delta);
}
void SyncSchedulerImpl::ForceSync() {
OnTimerFired();
}
base::TimeDelta SyncSchedulerImpl::GetTimeToNextSync() const {
if (!timer_)
return base::TimeDelta::FromSeconds(0);
return timer_->GetCurrentDelay();
}
SyncScheduler::Strategy SyncSchedulerImpl::GetStrategy() const {
return strategy_;
}
SyncScheduler::SyncState SyncSchedulerImpl::GetSyncState() const {
return sync_state_;
}
void SyncSchedulerImpl::OnTimerFired() {
timer_.reset();
if (strategy_ == Strategy::PERIODIC_REFRESH) {
PA_LOG(INFO) << "Timer fired for periodic refresh, making request...";
sync_state_ = SyncState::SYNC_IN_PROGRESS;
} else if (strategy_ == Strategy::AGGRESSIVE_RECOVERY) {
PA_LOG(INFO) << "Timer fired for aggressive recovery, making request...";
sync_state_ = SyncState::SYNC_IN_PROGRESS;
} else {
NOTREACHED();
return;
}
delegate_->OnSyncRequested(
make_scoped_ptr(new SyncRequest(weak_ptr_factory_.GetWeakPtr())));
}
scoped_ptr<base::Timer> SyncSchedulerImpl::CreateTimer() {
bool retain_user_task = false;
bool is_repeating = false;
return make_scoped_ptr(new base::Timer(retain_user_task, is_repeating));
}
void SyncSchedulerImpl::ScheduleNextSync(const base::TimeDelta& sync_delta) {
if (sync_state_ != SyncState::WAITING_FOR_REFRESH) {
PA_LOG(ERROR) << "Unexpected state when scheduling next sync: sync_state="
<< static_cast<int>(sync_state_);
return;
}
bool is_aggressive_recovery = (strategy_ == Strategy::AGGRESSIVE_RECOVERY);
PA_LOG(INFO) << "Scheduling next sync for " << scheduler_name_ << ":\n"
<< " Strategy: " << (is_aggressive_recovery
? "Aggressive Recovery"
: "Periodic Refresh") << "\n"
<< " Time Delta: " << TimeDeltaToString(sync_delta)
<< (is_aggressive_recovery
? base::StringPrintf(
"\n Previous Failures: %d",
base::saturated_cast<int>(failure_count_))
: "");
timer_ = CreateTimer();
timer_->Start(FROM_HERE, sync_delta,
base::Bind(&SyncSchedulerImpl::OnTimerFired,
weak_ptr_factory_.GetWeakPtr()));
}
void SyncSchedulerImpl::OnSyncCompleted(bool success) {
if (sync_state_ != SyncState::SYNC_IN_PROGRESS) {
PA_LOG(ERROR) << "Unexpected state when sync completed: sync_state="
<< static_cast<int>(sync_state_)
<< ", strategy_=" << static_cast<int>(strategy_);
return;
}
sync_state_ = SyncState::WAITING_FOR_REFRESH;
if (success) {
strategy_ = Strategy::PERIODIC_REFRESH;
failure_count_ = 0;
} else {
strategy_ = Strategy::AGGRESSIVE_RECOVERY;
++failure_count_;
}
ScheduleNextSync(GetJitteredPeriod());
}
base::TimeDelta SyncSchedulerImpl::GetJitteredPeriod() {
double jitter = 2 * max_jitter_ratio_ * (base::RandDouble() - 0.5);
base::TimeDelta period = GetPeriod();
base::TimeDelta jittered_time_delta = period + (period * jitter);
if (jittered_time_delta.InMilliseconds() < 0)
jittered_time_delta = base::TimeDelta::FromMilliseconds(0);
return jittered_time_delta;
}
base::TimeDelta SyncSchedulerImpl::GetPeriod() {
if (strategy_ == Strategy::PERIODIC_REFRESH) {
return refresh_period_;
} else if (strategy_ == Strategy::AGGRESSIVE_RECOVERY && failure_count_ > 0) {
// The backoff for each consecutive failure is exponentially doubled until
// it is equal to the normal refresh period.
// Note: |backoff_factor| may evaulate to INF if |failure_count_| is large,
// but multiplication operations for TimeDelta objects are saturated.
double backoff_factor = pow(2, failure_count_ - 1);
base::TimeDelta backoff_period = base_recovery_period_ * backoff_factor;
return backoff_period < refresh_period_ ? backoff_period : refresh_period_;
} else {
PA_LOG(ERROR) << "Error getting period for strategy: "
<< static_cast<int>(strategy_);
return base::TimeDelta();
}
}
} // namespace proximity_auth
components/proximity_auth/cryptauth/sync_scheduler_impl.h deleted 100644 → 0
View file @ cdc1058c
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_CRYPTAUTH_SYNC_SCHEDULER_IMPL_H
#define COMPONENTS_PROXIMITY_AUTH_CRYPTAUTH_CRYPTAUTH_SYNC_SCHEDULER_IMPL_H
#include "base/timer/timer.h"
#include "components/proximity_auth/cryptauth/sync_scheduler.h"
namespace proximity_auth {
// Implementation of SyncScheduler.
class SyncSchedulerImpl : public SyncScheduler {
public:
// Creates the scheduler:
// |delegate|: Handles sync requests and must outlive the scheduler.
// |refresh_period|: The time to wait for the PERIODIC_REFRESH strategy.
// |base_recovery_period|: The initial time to wait for the
// AGGRESSIVE_RECOVERY strategy. The time delta is increased for each
// subsequent failure.
// |max_jitter_ratio|: The maximum ratio that the time to next sync can be
// jittered (both positively and negatively).
// |scheduler_name|: The name of the scheduler for debugging purposes.
SyncSchedulerImpl(Delegate* delegate,
base::TimeDelta refresh_period,
base::TimeDelta base_recovery_period,
double max_jitter_ratio,
const std::string& scheduler_name);
~SyncSchedulerImpl() override;
// SyncScheduler:
void Start(const base::TimeDelta& elapsed_time_since_last_sync,
Strategy strategy) override;
void ForceSync() override;
base::TimeDelta GetTimeToNextSync() const override;
Strategy GetStrategy() const override;
SyncState GetSyncState() const override;
protected:
// Creates and returns a base::Timer object. Exposed for testing.
virtual scoped_ptr<base::Timer> CreateTimer();
private:
// SyncScheduler:
void OnSyncCompleted(bool success) override;
// Called when |timer_| is fired.
void OnTimerFired();
// Schedules |timer_| for the next sync request.
void ScheduleNextSync(const base::TimeDelta& sync_delta);
// Adds a random jitter to the value of GetPeriod(). The returned
// TimeDelta will be clamped to be non-negative.
base::TimeDelta GetJitteredPeriod();
// Returns the time to wait for the current strategy.
base::TimeDelta GetPeriod();
// The delegate handling sync requests when they are fired.
Delegate* const delegate_;
// The time to wait until the next refresh when the last sync attempt was
// successful.
const base::TimeDelta refresh_period_;
// The base recovery period for the AGGRESSIVE_RECOVERY strategy before
// backoffs are applied.
const base::TimeDelta base_recovery_period_;
// The maximum percentage (both positively and negatively) that the time to
// wait between each sync request is jittered. The jitter is randomly applied
// to each period so we can avoid synchronous calls to the server.
const double max_jitter_ratio_;
// The name of the scheduler, used for debugging purposes.
const std::string scheduler_name_;
// The current strategy of the scheduler.
Strategy strategy_;
// The current state of the scheduler.
SyncState sync_state_;
// The number of failed syncs made in a row. Once a sync request succeeds,
// this counter is reset.
size_t failure_count_;
// Timer firing for the next sync request.
scoped_ptr<base::Timer> timer_;
base::WeakPtrFactory<SyncSchedulerImpl> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(SyncSchedulerImpl);
};
} // namespace proximity_auth
#endif // COMPONENTS_PROXIMITY_CRYPTAUTH_CRYPTAUTH_SYNC_SCHEDULER_IMPL_H
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment