From f28a99a90b68e45f39191258832e7a526c4742ba Mon Sep 17 00:00:00 2001
From: Mathieu Chartier <mathieuc@google.com>
Date: Wed, 2 Mar 2016 10:30:23 -0800
Subject: [PATCH] Fix potential linear alloc memory leak

Previously, if we created a linear alloc for a class loader but
never created the class table, the linear alloc would never get
freed since it would have no corresponding ClassLoaderData.

Fixes valgrind-test-art-host-gtest-oat_test

Bug: 27384882
Bug: 22858531

(cherry picked from commit 5b83050affa6a3b1d3863c0b903f9d48fe4aefb2)

Change-Id: I71b650eac4e33212a7f03c43141db99e635a19ad
---
 runtime/class_linker.cc | 36 ++++++++++++++++++++++--------------
 runtime/class_linker.h  | 11 +++++++++--
 runtime/stack.cc        |  2 +-
 3 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 302fa0646..8228f4e7f 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2895,8 +2895,9 @@ LinearAlloc* ClassLinker::GetOrCreateAllocatorForClassLoader(mirror::ClassLoader
   WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
   LinearAlloc* allocator = class_loader->GetAllocator();
   if (allocator == nullptr) {
-    allocator = Runtime::Current()->CreateLinearAlloc();
-    class_loader->SetAllocator(allocator);
+    RegisterClassLoader(class_loader);
+    allocator = class_loader->GetAllocator();
+    CHECK(allocator != nullptr);
   }
   return allocator;
 }
@@ -4857,24 +4858,31 @@ void ClassLinker::FixupTemporaryDeclaringClass(mirror::Class* temp_class,
   Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(new_class);
 }
 
+void ClassLinker::RegisterClassLoader(mirror::ClassLoader* class_loader) {
+  CHECK(class_loader->GetAllocator() == nullptr);
+  CHECK(class_loader->GetClassTable() == nullptr);
+  Thread* const self = Thread::Current();
+  ClassLoaderData data;
+  data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader);
+  // Create and set the class table.
+  data.class_table = new ClassTable;
+  class_loader->SetClassTable(data.class_table);
+  // Create and set the linear allocator.
+  data.allocator = Runtime::Current()->CreateLinearAlloc();
+  class_loader->SetAllocator(data.allocator);
+  // Add to the list so that we know to free the data later.
+  class_loaders_.push_back(data);
+}
+
 ClassTable* ClassLinker::InsertClassTableForClassLoader(mirror::ClassLoader* class_loader) {
   if (class_loader == nullptr) {
     return &boot_class_table_;
   }
   ClassTable* class_table = class_loader->GetClassTable();
   if (class_table == nullptr) {
-    class_table = new ClassTable;
-    Thread* const self = Thread::Current();
-    ClassLoaderData data;
-    data.weak_root = self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader);
-    data.class_table = class_table;
-    // Don't already have a class table, add it to the class loader.
-    CHECK(class_loader->GetClassTable() == nullptr);
-    class_loader->SetClassTable(data.class_table);
-    // Should have been set when we registered the dex file.
-    data.allocator = class_loader->GetAllocator();
-    CHECK(data.allocator != nullptr);
-    class_loaders_.push_back(data);
+    RegisterClassLoader(class_loader);
+    class_table = class_loader->GetClassTable();
+    DCHECK(class_table != nullptr);
   }
   return class_table;
 }
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index a6a0cc12f..452edbbdf 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -582,12 +582,12 @@ class ClassLinker {
 
   // Unlike GetOrCreateAllocatorForClassLoader, GetAllocatorForClassLoader asserts that the
   // allocator for this class loader is already created.
-  static LinearAlloc* GetAllocatorForClassLoader(mirror::ClassLoader* class_loader)
+  LinearAlloc* GetAllocatorForClassLoader(mirror::ClassLoader* class_loader)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
   // Return the linear alloc for a class loader if it is already allocated, otherwise allocate and
   // set it. TODO: Consider using a lock other than classlinker_classes_lock_.
-  static LinearAlloc* GetOrCreateAllocatorForClassLoader(mirror::ClassLoader* class_loader)
+  LinearAlloc* GetOrCreateAllocatorForClassLoader(mirror::ClassLoader* class_loader)
       REQUIRES(!Locks::classlinker_classes_lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
@@ -986,9 +986,16 @@ class ClassLinker {
   mirror::Class* LookupClassFromBootImage(const char* descriptor)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
+  // Register a class loader and create its class table and allocator. Should not be called if
+  // these are already created.
+  void RegisterClassLoader(mirror::ClassLoader* class_loader)
+      SHARED_REQUIRES(Locks::mutator_lock_)
+      REQUIRES(Locks::classlinker_classes_lock_);
+
   // Returns null if not found.
   ClassTable* ClassTableForClassLoader(mirror::ClassLoader* class_loader)
       SHARED_REQUIRES(Locks::mutator_lock_, Locks::classlinker_classes_lock_);
+
   // Insert a new class table if not found.
   ClassTable* InsertClassTableForClassLoader(mirror::ClassLoader* class_loader)
       SHARED_REQUIRES(Locks::mutator_lock_)
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 5faff93b9..342273498 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -740,7 +740,7 @@ void StackVisitor::SanityCheckFrame() const {
       // Check class linker linear allocs.
       mirror::Class* klass = method->GetDeclaringClass();
       LinearAlloc* const class_linear_alloc = (klass != nullptr)
-          ? ClassLinker::GetAllocatorForClassLoader(klass->GetClassLoader())
+          ? runtime->GetClassLinker()->GetAllocatorForClassLoader(klass->GetClassLoader())
           : linear_alloc;
       if (!class_linear_alloc->Contains(method)) {
         // Check image space.
-- 
2.11.0