Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(298)

Unified Diff: components/leveldb/env_mojo.cc

Issue 2722293002: Fix lifetime of leveldb::MojoEnv instances. (Closed)
Patch Set: annotate leaks Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/leveldb/env_mojo.cc
diff --git a/components/leveldb/env_mojo.cc b/components/leveldb/env_mojo.cc
index 82af93a997036fa73be861228083b196b93f85f9..178f7672747484dde25f81ebe2af1e5ad5d829ce 100644
--- a/components/leveldb/env_mojo.cc
+++ b/components/leveldb/env_mojo.cc
@@ -8,7 +8,10 @@
#include <memory>
+#include "base/lazy_instance.h"
+#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
#include "base/trace_event/trace_event.h"
#include "third_party/leveldatabase/chromium_logger.h"
#include "third_party/leveldatabase/src/include/leveldb/status.h"
@@ -217,20 +220,35 @@ class MojoWritableFile : public leveldb::WritableFile {
} // namespace
-MojoEnv::MojoEnv(const std::string& name,
- scoped_refptr<LevelDBMojoProxy> file_thread,
- LevelDBMojoProxy::OpaqueDir* dir)
- : ChromiumEnv(name), thread_(file_thread), dir_(dir) {}
+MojoEnv::Directory::~Directory() {
+ env_->UnregisterDirectory(prefix_);
+}
+
+MojoEnv::Directory::Directory(MojoEnv* env, std::string prefix)
+ : env_(env), prefix_(std::move(prefix)) {}
-MojoEnv::~MojoEnv() {
- thread_->UnregisterDirectory(dir_);
+MojoEnv::MojoEnv(scoped_refptr<base::SingleThreadTaskRunner> file_task_runner)
+ : ChromiumEnv("LevelDBEnv.Mojo"),
+ thread_(new LevelDBMojoProxy(std::move(file_task_runner))) {}
+
+std::unique_ptr<MojoEnv::Directory> MojoEnv::RegisterDirectory(
+ filesystem::mojom::DirectoryPtr directory) {
+ LevelDBMojoProxy::OpaqueDir* dir =
+ thread_->RegisterDirectory(std::move(directory));
+ int dir_id = dirs_.Add(dir);
+ std::string prefix = base::StringPrintf("%d:", dir_id);
+ return base::WrapUnique(new Directory(this, prefix));
}
Status MojoEnv::NewSequentialFile(const std::string& fname,
SequentialFile** result) {
TRACE_EVENT1("leveldb", "MojoEnv::NewSequentialFile", "fname", fname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
base::File f = thread_->OpenFileHandle(
- dir_, fname, filesystem::mojom::kFlagOpen | filesystem::mojom::kFlagRead);
+ dir, path, filesystem::mojom::kFlagOpen | filesystem::mojom::kFlagRead);
if (!f.IsValid()) {
*result = nullptr;
return MakeIOError(fname, "Unable to create sequential file",
@@ -244,105 +262,152 @@ Status MojoEnv::NewSequentialFile(const std::string& fname,
Status MojoEnv::NewRandomAccessFile(const std::string& fname,
RandomAccessFile** result) {
TRACE_EVENT1("leveldb", "MojoEnv::NewRandomAccessFile", "fname", fname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
base::File f = thread_->OpenFileHandle(
- dir_, fname, filesystem::mojom::kFlagRead | filesystem::mojom::kFlagOpen);
+ dir, path, filesystem::mojom::kFlagRead | filesystem::mojom::kFlagOpen);
if (!f.IsValid()) {
*result = nullptr;
base::File::Error error_code = f.error_details();
- return MakeIOError(fname, FileErrorString(error_code),
+ return MakeIOError(path, FileErrorString(error_code),
leveldb_env::kNewRandomAccessFile, error_code);
}
- *result = new MojoRandomAccessFile(fname, std::move(f));
+ *result = new MojoRandomAccessFile(path, std::move(f));
return Status::OK();
}
Status MojoEnv::NewWritableFile(const std::string& fname,
WritableFile** result) {
TRACE_EVENT1("leveldb", "MojoEnv::NewWritableFile", "fname", fname);
- base::File f =
- thread_->OpenFileHandle(dir_, fname, filesystem::mojom::kCreateAlways |
- filesystem::mojom::kFlagWrite);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
+ base::File f = thread_->OpenFileHandle(
+ dir, path,
+ filesystem::mojom::kCreateAlways | filesystem::mojom::kFlagWrite);
if (!f.IsValid()) {
*result = nullptr;
- return MakeIOError(fname, "Unable to create writable file",
+ return MakeIOError(path, "Unable to create writable file",
leveldb_env::kNewWritableFile, f.error_details());
}
- *result = new MojoWritableFile(dir_, fname, std::move(f), thread_);
+ *result = new MojoWritableFile(dir, path, std::move(f), thread_);
return Status::OK();
}
Status MojoEnv::NewAppendableFile(const std::string& fname,
WritableFile** result) {
TRACE_EVENT1("leveldb", "MojoEnv::NewAppendableFile", "fname", fname);
- base::File f =
- thread_->OpenFileHandle(dir_, fname, filesystem::mojom::kFlagOpenAlways |
- filesystem::mojom::kFlagAppend);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
+ base::File f = thread_->OpenFileHandle(
+ dir, path,
+ filesystem::mojom::kFlagOpenAlways | filesystem::mojom::kFlagAppend);
if (!f.IsValid()) {
*result = nullptr;
- return MakeIOError(fname, "Unable to create appendable file",
+ return MakeIOError(path, "Unable to create appendable file",
leveldb_env::kNewAppendableFile, f.error_details());
}
- *result = new MojoWritableFile(dir_, fname, std::move(f), thread_);
+ *result = new MojoWritableFile(dir, path, std::move(f), thread_);
return Status::OK();
}
bool MojoEnv::FileExists(const std::string& fname) {
TRACE_EVENT1("leveldb", "MojoEnv::FileExists", "fname", fname);
- return thread_->FileExists(dir_, fname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
+ return thread_->FileExists(dir, path);
}
-Status MojoEnv::GetChildren(const std::string& path,
+Status MojoEnv::GetChildren(const std::string& dirname,
std::vector<std::string>* result) {
- TRACE_EVENT1("leveldb", "MojoEnv::GetChildren", "path", path);
- return FilesystemErrorToStatus(thread_->GetChildren(dir_, path, result), path,
+ TRACE_EVENT1("leveldb", "MojoEnv::GetChildren", "dirname", dirname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(dirname, &dir, &path);
+
+ return FilesystemErrorToStatus(thread_->GetChildren(dir, path, result), path,
leveldb_env::kGetChildren);
}
Status MojoEnv::DeleteFile(const std::string& fname) {
TRACE_EVENT1("leveldb", "MojoEnv::DeleteFile", "fname", fname);
- return FilesystemErrorToStatus(thread_->Delete(dir_, fname, 0), fname,
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
+ return FilesystemErrorToStatus(thread_->Delete(dir, path, 0), path,
leveldb_env::kDeleteFile);
}
Status MojoEnv::CreateDir(const std::string& dirname) {
TRACE_EVENT1("leveldb", "MojoEnv::CreateDir", "dirname", dirname);
- return FilesystemErrorToStatus(thread_->CreateDir(dir_, dirname), dirname,
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(dirname, &dir, &path);
+
+ return FilesystemErrorToStatus(thread_->CreateDir(dir, path), path,
leveldb_env::kCreateDir);
}
Status MojoEnv::DeleteDir(const std::string& dirname) {
TRACE_EVENT1("leveldb", "MojoEnv::DeleteDir", "dirname", dirname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(dirname, &dir, &path);
+
return FilesystemErrorToStatus(
- thread_->Delete(dir_, dirname, filesystem::mojom::kDeleteFlagRecursive),
- dirname, leveldb_env::kDeleteDir);
+ thread_->Delete(dir, path, filesystem::mojom::kDeleteFlagRecursive), path,
+ leveldb_env::kDeleteDir);
}
Status MojoEnv::GetFileSize(const std::string& fname, uint64_t* file_size) {
TRACE_EVENT1("leveldb", "MojoEnv::GetFileSize", "fname", fname);
- return FilesystemErrorToStatus(thread_->GetFileSize(dir_, fname, file_size),
- fname,
- leveldb_env::kGetFileSize);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
+ return FilesystemErrorToStatus(thread_->GetFileSize(dir, path, file_size),
+ path, leveldb_env::kGetFileSize);
}
Status MojoEnv::RenameFile(const std::string& src, const std::string& target) {
TRACE_EVENT2("leveldb", "MojoEnv::RenameFile", "src", src, "target", target);
- return FilesystemErrorToStatus(thread_->RenameFile(dir_, src, target), src,
- leveldb_env::kRenameFile);
+ LevelDBMojoProxy::OpaqueDir* src_dir;
+ std::string src_path;
+ ParsePath(src, &src_dir, &src_path);
+ LevelDBMojoProxy::OpaqueDir* target_dir;
+ std::string target_path;
+ ParsePath(target, &target_dir, &target_path);
+ DCHECK_EQ(src_dir, target_dir);
+
+ return FilesystemErrorToStatus(
+ thread_->RenameFile(src_dir, src_path, target_path), src_path,
+ leveldb_env::kRenameFile);
}
Status MojoEnv::LockFile(const std::string& fname, FileLock** lock) {
TRACE_EVENT1("leveldb", "MojoEnv::LockFile", "fname", fname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
std::pair<filesystem::mojom::FileError, LevelDBMojoProxy::OpaqueLock*> p =
- thread_->LockFile(dir_, fname);
+ thread_->LockFile(dir, path);
if (p.second)
*lock = new MojoFileLock(p.second, fname);
- return FilesystemErrorToStatus(p.first, fname, leveldb_env::kLockFile);
+ return FilesystemErrorToStatus(p.first, path, leveldb_env::kLockFile);
}
Status MojoEnv::UnlockFile(FileLock* lock) {
@@ -368,12 +433,16 @@ Status MojoEnv::GetTestDirectory(std::string* path) {
Status MojoEnv::NewLogger(const std::string& fname, Logger** result) {
TRACE_EVENT1("leveldb", "MojoEnv::NewLogger", "fname", fname);
+ LevelDBMojoProxy::OpaqueDir* dir;
+ std::string path;
+ ParsePath(fname, &dir, &path);
+
base::File f(thread_->OpenFileHandle(
- dir_, fname,
+ dir, path,
filesystem::mojom::kCreateAlways | filesystem::mojom::kFlagWrite));
if (!f.IsValid()) {
*result = NULL;
- return MakeIOError(fname, "Unable to create log file",
+ return MakeIOError(path, "Unable to create log file",
leveldb_env::kNewLogger, f.error_details());
} else {
*result = new leveldb::ChromiumLogger(std::move(f));
@@ -381,4 +450,35 @@ Status MojoEnv::NewLogger(const std::string& fname, Logger** result) {
}
}
+MojoEnv::~MojoEnv() {}
+
+void MojoEnv::ParsePath(const std::string& path,
+ LevelDBMojoProxy::OpaqueDir** out_dir,
+ std::string* out_path) {
+ size_t colon_pos = path.find(':');
+ DCHECK_NE(colon_pos, std::string::npos);
+
+ int dir_id;
+ bool result = base::StringToInt(
+ base::StringPiece(path.begin(), path.begin() + colon_pos), &dir_id);
+ DCHECK(result) << "Invalid directory id in path '" << path << "'";
+ *out_dir = dirs_.Lookup(dir_id);
+ DCHECK(*out_dir) << "Invalid directory id " << dir_id;
+
+ *out_path = std::string(path.begin() + colon_pos + 1, path.end());
+}
+
+void MojoEnv::UnregisterDirectory(base::StringPiece prefix) {
+ DCHECK(!prefix.empty());
+ DCHECK_EQ(prefix.back(), ':');
+ prefix.remove_suffix(1);
+ int dir_id;
+ bool result = base::StringToInt(prefix, &dir_id);
+ DCHECK(result) << "Invalid directory id '" << prefix << "'";
+ LevelDBMojoProxy::OpaqueDir* dir = dirs_.Lookup(dir_id);
+ DCHECK(dir) << "Invalid directory id " << dir_id;
+ dirs_.Remove(dir_id);
+ thread_->UnregisterDirectory(dir);
+}
+
} // namespace leveldb

Powered by Google App Engine
This is Rietveld 408576698