Commit 01f313e6 authored by Salvador Guerrero's avatar Salvador Guerrero Committed by Commit Bot

Fixed Destroy method on unique proto databases that failed to initialize

Some leveldb_proto clients attempt to destroy their database when it
fails to initialize, this would fail because we needed to have an
initialized instance in order to destroy it.

This change calls the static method leveldb::DestroyDB which will try
to erase the database directory. This will ensure that clients using a
DB selector will be able to recover from a failed initialization

Renamed wrapper to DB impl on proto_database_impl_unittest to match new
class name

Bug: 870813
Change-Id: I9cbf4482cde132f68abeecb3b210c289bba0b1b2
Reviewed-on: https://chromium-review.googlesource.com/c/1484086
Commit-Queue: Salvador Guerrero <salg@google.com>
Reviewed-by: default avatarssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635839}
parent d8fc62f5
...@@ -51,6 +51,8 @@ void ProtoDatabaseSelector::InitWithDatabase( ...@@ -51,6 +51,8 @@ void ProtoDatabaseSelector::InitWithDatabase(
if (!db_) if (!db_)
db_ = std::make_unique<UniqueProtoDatabase>(task_runner_); db_ = std::make_unique<UniqueProtoDatabase>(task_runner_);
unique_database_dir_ = database_dir;
db_->InitWithDatabase( db_->InitWithDatabase(
database, database_dir, options, false, database, database_dir, options, false,
base::BindOnce(&RunInitCallbackOnTaskRunner, std::move(callback), base::BindOnce(&RunInitCallbackOnTaskRunner, std::move(callback),
...@@ -67,16 +69,17 @@ void ProtoDatabaseSelector::InitUniqueOrShared( ...@@ -67,16 +69,17 @@ void ProtoDatabaseSelector::InitUniqueOrShared(
Callbacks::InitStatusCallback callback) { Callbacks::InitStatusCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
init_status_ = InitStatus::IN_PROGRESS; init_status_ = InitStatus::IN_PROGRESS;
unique_database_dir_ = db_dir;
client_name_ = client_name;
auto unique_db = auto unique_db =
std::make_unique<UniqueProtoDatabase>(db_dir, options, task_runner_); std::make_unique<UniqueProtoDatabase>(db_dir, options, task_runner_);
auto* unique_db_ptr = unique_db.get(); auto* unique_db_ptr = unique_db.get();
unique_db_ptr->Init( unique_db_ptr->Init(
client_name.c_str(), client_name, base::BindOnce(&ProtoDatabaseSelector::OnInitUniqueDB, this,
base::BindOnce( std::move(unique_db), use_shared_db,
&ProtoDatabaseSelector::OnInitUniqueDB, this, std::move(unique_db), base::BindOnce(&RunInitCallbackOnTaskRunner,
use_shared_db, std::move(callback),
base::BindOnce(&RunInitCallbackOnTaskRunner, std::move(callback), callback_task_runner)));
callback_task_runner)));
} }
void ProtoDatabaseSelector::OnInitUniqueDB( void ProtoDatabaseSelector::OnInitUniqueDB(
...@@ -535,9 +538,16 @@ void ProtoDatabaseSelector::GetEntry(const std::string& key, ...@@ -535,9 +538,16 @@ void ProtoDatabaseSelector::GetEntry(const std::string& key,
void ProtoDatabaseSelector::Destroy(Callbacks::DestroyCallback callback) { void ProtoDatabaseSelector::Destroy(Callbacks::DestroyCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!db_) { if (!db_) {
if (!unique_database_dir_.empty()) {
ProtoLevelDBWrapper::Destroy(unique_database_dir_, client_name_,
task_runner_, std::move(callback));
return;
}
std::move(callback).Run(false); std::move(callback).Run(false);
return; return;
} }
db_->Destroy(std::move(callback)); db_->Destroy(std::move(callback));
} }
......
...@@ -161,6 +161,8 @@ class ProtoDatabaseSelector ...@@ -161,6 +161,8 @@ class ProtoDatabaseSelector
InitStatus init_status_ = InitStatus::NOT_STARTED; InitStatus init_status_ = InitStatus::NOT_STARTED;
base::queue<base::OnceClosure> pending_tasks_; base::queue<base::OnceClosure> pending_tasks_;
std::unique_ptr<UniqueProtoDatabase> db_; std::unique_ptr<UniqueProtoDatabase> db_;
base::FilePath unique_database_dir_;
std::string client_name_;
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
}; };
......
...@@ -44,6 +44,18 @@ bool DestroyFromTaskRunner(LevelDB* database, const std::string& client_id) { ...@@ -44,6 +44,18 @@ bool DestroyFromTaskRunner(LevelDB* database, const std::string& client_id) {
return success; return success;
} }
bool DestroyWithDirectoryFromTaskRunner(const base::FilePath& db_dir,
const std::string& client_id) {
leveldb::Status result = leveldb::DestroyDB(
db_dir.AsUTF8Unsafe(), leveldb_proto::CreateSimpleOptions());
bool success = result.ok();
if (!client_id.empty())
ProtoLevelDBWrapperMetrics::RecordDestroy(client_id, success);
return success;
}
void LoadKeysFromTaskRunner( void LoadKeysFromTaskRunner(
LevelDB* database, LevelDB* database,
const std::string& target_prefix, const std::string& target_prefix,
...@@ -376,6 +388,17 @@ void ProtoLevelDBWrapper::Destroy(Callbacks::DestroyCallback callback) { ...@@ -376,6 +388,17 @@ void ProtoLevelDBWrapper::Destroy(Callbacks::DestroyCallback callback) {
std::move(callback)); std::move(callback));
} }
void ProtoLevelDBWrapper::Destroy(
const base::FilePath& db_dir,
const std::string& client_id,
const scoped_refptr<base::SequencedTaskRunner>& task_runner,
Callbacks::DestroyCallback callback) {
base::PostTaskAndReplyWithResult(
task_runner.get(), FROM_HERE,
base::BindOnce(DestroyWithDirectoryFromTaskRunner, db_dir, client_id),
std::move(callback));
}
void ProtoLevelDBWrapper::SetMetricsId(const std::string& id) { void ProtoLevelDBWrapper::SetMetricsId(const std::string& id) {
metrics_id_ = id; metrics_id_ = id;
} }
......
...@@ -40,6 +40,13 @@ using ValueVector = std::vector<std::string>; ...@@ -40,6 +40,13 @@ using ValueVector = std::vector<std::string>;
// Construction/calls/destruction should all happen on the same thread. // Construction/calls/destruction should all happen on the same thread.
class ProtoLevelDBWrapper { class ProtoLevelDBWrapper {
public: public:
// Used to destroy database when initialization fails.
static void Destroy(
const base::FilePath& db_dir,
const std::string& client_id,
const scoped_refptr<base::SequencedTaskRunner>& task_runner,
Callbacks::DestroyCallback callback);
// All blocking calls/disk access will happen on the provided |task_runner|. // All blocking calls/disk access will happen on the provided |task_runner|.
ProtoLevelDBWrapper( ProtoLevelDBWrapper(
const scoped_refptr<base::SequencedTaskRunner>& task_runner); const scoped_refptr<base::SequencedTaskRunner>& task_runner);
......
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