0

leveldb: Have GetOrCreateAllocatorDump fail on untracked db.

The callers of DBTracker::GetOrCreateAllocatorDump assume that the
DBTracker is dumping the passed |db|. Adding a DCHECK to detect
adding an edge to a nonexistent dump.

BUG=711518

Change-Id: I5b6fc966eb56d97436add9b24117f6b4539c2690
Reviewed-on: https://chromium-review.googlesource.com/650838
Commit-Queue: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502019}
This commit is contained in:
Chris Mumford
2017-09-14 19:45:46 +00:00
committed by Commit Bot
parent 14e8ff0851
commit 33b762b8f0
3 changed files with 70 additions and 12 deletions

@@ -1338,6 +1338,7 @@ DBTracker::~DBTracker() {
NOTREACHED(); // DBTracker is a singleton NOTREACHED(); // DBTracker is a singleton
} }
// static
DBTracker* DBTracker::GetInstance() { DBTracker* DBTracker::GetInstance() {
static DBTracker* instance = new DBTracker(); static DBTracker* instance = new DBTracker();
return instance; return instance;
@@ -1347,12 +1348,22 @@ DBTracker* DBTracker::GetInstance() {
base::trace_event::MemoryAllocatorDump* DBTracker::GetOrCreateAllocatorDump( base::trace_event::MemoryAllocatorDump* DBTracker::GetOrCreateAllocatorDump(
base::trace_event::ProcessMemoryDump* pmd, base::trace_event::ProcessMemoryDump* pmd,
leveldb::DB* tracked_db) { leveldb::DB* tracked_db) {
DCHECK(GetInstance()->IsTrackedDB(tracked_db))
<< std::hex << tracked_db << " is not tracked";
return GetOrCreateAllocatorDump(pmd,
reinterpret_cast<TrackedDB*>(tracked_db));
}
// static
base::trace_event::MemoryAllocatorDump* DBTracker::GetOrCreateAllocatorDump(
base::trace_event::ProcessMemoryDump* pmd,
TrackedDB* db) {
if (pmd->dump_args().level_of_detail == if (pmd->dump_args().level_of_detail ==
base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) { base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND) {
return nullptr; return nullptr;
} }
std::string dump_name = base::StringPrintf( std::string dump_name = base::StringPrintf("leveldatabase/0x%" PRIXPTR,
"leveldatabase/0x%" PRIXPTR, reinterpret_cast<uintptr_t>(tracked_db)); reinterpret_cast<uintptr_t>(db));
auto* dump = pmd->GetAllocatorDump(dump_name); auto* dump = pmd->GetAllocatorDump(dump_name);
if (dump) if (dump)
return dump; return dump;
@@ -1360,9 +1371,9 @@ base::trace_event::MemoryAllocatorDump* DBTracker::GetOrCreateAllocatorDump(
uint64_t memory_usage = 0; uint64_t memory_usage = 0;
std::string usage_string; std::string usage_string;
bool success = tracked_db->GetProperty("leveldb.approximate-memory-usage", bool success =
&usage_string) && db->GetProperty("leveldb.approximate-memory-usage", &usage_string) &&
base::StringToUint64(usage_string, &memory_usage); base::StringToUint64(usage_string, &memory_usage);
DCHECK(success); DCHECK(success);
dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize, dump->AddScalar(base::trace_event::MemoryAllocatorDump::kNameSize,
base::trace_event::MemoryAllocatorDump::kUnitsBytes, base::trace_event::MemoryAllocatorDump::kUnitsBytes,
@@ -1376,6 +1387,15 @@ base::trace_event::MemoryAllocatorDump* DBTracker::GetOrCreateAllocatorDump(
return dump; return dump;
} }
bool DBTracker::IsTrackedDB(const leveldb::DB* db) const {
base::AutoLock lock(databases_lock_);
for (auto* i = databases_.head(); i != databases_.end(); i = i->next()) {
if (i->value() == db)
return true;
}
return false;
}
leveldb::Status DBTracker::OpenDatabase(const leveldb::Options& options, leveldb::Status DBTracker::OpenDatabase(const leveldb::Options& options,
const std::string& name, const std::string& name,
TrackedDB** dbptr) { TrackedDB** dbptr) {

@@ -15,6 +15,7 @@
#include "base/containers/linked_list.h" #include "base/containers/linked_list.h"
#include "base/files/file.h" #include "base/files/file.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/metrics/histogram.h" #include "base/metrics/histogram.h"
#include "leveldb/db.h" #include "leveldb/db.h"
@@ -280,8 +281,22 @@ class DBTracker {
const std::string& name, const std::string& name,
TrackedDB** dbptr); TrackedDB** dbptr);
private:
class MemoryDumpProvider;
class TrackedDBImpl;
using DatabaseVisitor = base::RepeatingCallback<void(TrackedDB*)>; using DatabaseVisitor = base::RepeatingCallback<void(TrackedDB*)>;
friend class ChromiumEnvDBTrackerTest;
FRIEND_TEST_ALL_PREFIXES(ChromiumEnvDBTrackerTest, IsTrackedDB);
DBTracker();
~DBTracker();
static base::trace_event::MemoryAllocatorDump* GetOrCreateAllocatorDump(
base::trace_event::ProcessMemoryDump* pmd,
TrackedDB* db);
// Calls |visitor| for each live database. The database is live from the // Calls |visitor| for each live database. The database is live from the
// point it was returned from OpenDatabase() and up until its instance is // point it was returned from OpenDatabase() and up until its instance is
// destroyed. // destroyed.
@@ -290,19 +305,15 @@ class DBTracker {
// destroyed (but doesn't lock the databases themselves). // destroyed (but doesn't lock the databases themselves).
void VisitDatabases(const DatabaseVisitor& visitor); void VisitDatabases(const DatabaseVisitor& visitor);
private: // Checks if |db| is tracked.
class TrackedDBImpl; bool IsTrackedDB(const leveldb::DB* db) const;
class MemoryDumpProvider;
DBTracker();
~DBTracker();
void DatabaseOpened(TrackedDBImpl* database); void DatabaseOpened(TrackedDBImpl* database);
void DatabaseDestroyed(TrackedDBImpl* database); void DatabaseDestroyed(TrackedDBImpl* database);
std::unique_ptr<MemoryDumpProvider> mdp_; std::unique_ptr<MemoryDumpProvider> mdp_;
base::Lock databases_lock_; mutable base::Lock databases_lock_;
base::LinkedList<TrackedDBImpl> databases_; base::LinkedList<TrackedDBImpl> databases_;
DISALLOW_COPY_AND_ASSIGN(DBTracker); DISALLOW_COPY_AND_ASSIGN(DBTracker);

@@ -33,6 +33,8 @@ using leveldb_env::DBTracker;
using leveldb_env::MethodID; using leveldb_env::MethodID;
using leveldb_env::Options; using leveldb_env::Options;
namespace leveldb_env {
static const int kReadOnlyFileLimit = 4; static const int kReadOnlyFileLimit = 4;
TEST(ErrorEncoding, OnlyAMethod) { TEST(ErrorEncoding, OnlyAMethod) {
@@ -381,4 +383,29 @@ TEST_F(ChromiumEnvDBTrackerTest, OpenDBTracking) {
ASSERT_EQ(db.get(), *visited_dbs.begin()); ASSERT_EQ(db.get(), *visited_dbs.begin());
} }
TEST_F(ChromiumEnvDBTrackerTest, IsTrackedDB) {
leveldb_env::Options options;
options.create_if_missing = true;
leveldb::DB* untracked_db;
base::ScopedTempDir untracked_temp_dir;
ASSERT_TRUE(untracked_temp_dir.CreateUniqueTempDir());
leveldb::Status s = leveldb::DB::Open(
options, untracked_temp_dir.GetPath().AsUTF8Unsafe(), &untracked_db);
ASSERT_TRUE(s.ok());
EXPECT_FALSE(DBTracker::GetInstance()->IsTrackedDB(untracked_db));
// Now a tracked db.
std::unique_ptr<leveldb::DB> tracked_db;
base::ScopedTempDir tracked_temp_dir;
ASSERT_TRUE(tracked_temp_dir.CreateUniqueTempDir());
s = leveldb_env::OpenDB(options, tracked_temp_dir.GetPath().AsUTF8Unsafe(),
&tracked_db);
ASSERT_TRUE(s.ok());
EXPECT_TRUE(DBTracker::GetInstance()->IsTrackedDB(tracked_db.get()));
delete untracked_db;
}
} // namespace leveldb_env
int main(int argc, char** argv) { return base::TestSuite(argc, argv).Run(); } int main(int argc, char** argv) { return base::TestSuite(argc, argv).Run(); }