Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a37bac79 authored by Yong Li's avatar Yong Li Committed by Automerger Merge Worker
Browse files

Fix memory leak of DexFile handle after release am: 4ad53837

Change-Id: Ida8e84793625820d95602815fbbb7f25962c5f88
parents 1ea41439 4ad53837
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -89,7 +89,7 @@ std::unique_ptr<DexFileFromFile> DexFileFromFile::Create(uint64_t dex_file_offse
    return nullptr;
  }

  return std::unique_ptr<DexFileFromFile>(new DexFileFromFile(std::move(*art_dex_file.release())));
  return std::unique_ptr<DexFileFromFile>(new DexFileFromFile(art_dex_file));
}

std::unique_ptr<DexFileFromMemory> DexFileFromMemory::Create(uint64_t dex_file_offset_in_memory,
@@ -108,7 +108,7 @@ std::unique_ptr<DexFileFromMemory> DexFileFromMemory::Create(uint64_t dex_file_o

    if (art_dex_file != nullptr) {
      return std::unique_ptr<DexFileFromMemory>(
          new DexFileFromMemory(std::move(*art_dex_file.release()), std::move(backing_memory)));
          new DexFileFromMemory(art_dex_file, std::move(backing_memory)));
    }

    if (!error_msg.empty()) {
+6 −4
Original line number Diff line number Diff line
@@ -39,7 +39,8 @@ class DexFile : protected art_api::dex::DexFile {
                                         MapInfo* info);

 protected:
  DexFile(art_api::dex::DexFile&& art_dex_file) : art_api::dex::DexFile(std::move(art_dex_file)) {}
  DexFile(std::unique_ptr<art_api::dex::DexFile>& art_dex_file)
      : art_api::dex::DexFile(art_dex_file) {}
};

class DexFileFromFile : public DexFile {
@@ -48,7 +49,7 @@ class DexFileFromFile : public DexFile {
                                                 const std::string& file);

 private:
  DexFileFromFile(art_api::dex::DexFile&& art_dex_file) : DexFile(std::move(art_dex_file)) {}
  DexFileFromFile(std::unique_ptr<art_api::dex::DexFile>& art_dex_file) : DexFile(art_dex_file) {}
};

class DexFileFromMemory : public DexFile {
@@ -57,8 +58,9 @@ class DexFileFromMemory : public DexFile {
                                                   Memory* memory, const std::string& name);

 private:
  DexFileFromMemory(art_api::dex::DexFile&& art_dex_file, std::vector<uint8_t>&& memory)
      : DexFile(std::move(art_dex_file)), memory_(std::move(memory)) {}
  DexFileFromMemory(std::unique_ptr<art_api::dex::DexFile>& art_dex_file,
                    std::vector<uint8_t>&& memory)
      : DexFile(art_dex_file), memory_(std::move(memory)) {}

  std::vector<uint8_t> memory_;
};
+45 −0
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@
 * limitations under the License.
 */

#include <malloc.h>
#include <stdint.h>
#include <sys/types.h>
#include <unistd.h>
@@ -72,6 +73,37 @@ TEST(DexFileTest, from_file_open_non_zero_offset) {
  EXPECT_TRUE(DexFileFromFile::Create(0x100, tf.path) != nullptr);
}

static constexpr size_t kNumLeakLoops = 5000;
static constexpr size_t kMaxAllowedLeakBytes = 1024;

static void CheckForLeak(size_t loop, size_t* first_allocated_bytes, size_t* last_allocated_bytes) {
  size_t allocated_bytes = mallinfo().uordblks;
  if (*first_allocated_bytes == 0) {
    *first_allocated_bytes = allocated_bytes;
  } else if (*last_allocated_bytes > *first_allocated_bytes) {
    // Check that the total memory did not increase too much over the first loop.
    ASSERT_LE(*last_allocated_bytes - *first_allocated_bytes, kMaxAllowedLeakBytes)
        << "Failed in loop " << loop << " first_allocated_bytes " << *first_allocated_bytes
        << " last_allocated_bytes " << *last_allocated_bytes;
  }
  *last_allocated_bytes = allocated_bytes;
}

TEST(DexFileTest, from_file_no_leak) {
  TemporaryFile tf;
  ASSERT_TRUE(tf.fd != -1);

  ASSERT_EQ(sizeof(kDexData),
            static_cast<size_t>(TEMP_FAILURE_RETRY(write(tf.fd, kDexData, sizeof(kDexData)))));

  size_t first_allocated_bytes = 0;
  size_t last_allocated_bytes = 0;
  for (size_t i = 0; i < kNumLeakLoops; i++) {
    EXPECT_TRUE(DexFileFromFile::Create(0, tf.path) != nullptr);
    ASSERT_NO_FATAL_FAILURE(CheckForLeak(i, &first_allocated_bytes, &last_allocated_bytes));
  }
}

TEST(DexFileTest, from_memory_fail_too_small_for_header) {
  MemoryFake memory;

@@ -96,6 +128,19 @@ TEST(DexFileTest, from_memory_open) {
  EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr);
}

TEST(DexFileTest, from_memory_no_leak) {
  MemoryFake memory;

  memory.SetMemory(0x1000, kDexData, sizeof(kDexData));

  size_t first_allocated_bytes = 0;
  size_t last_allocated_bytes = 0;
  for (size_t i = 0; i < kNumLeakLoops; i++) {
    EXPECT_TRUE(DexFileFromMemory::Create(0x1000, &memory, "") != nullptr);
    ASSERT_NO_FATAL_FAILURE(CheckForLeak(i, &first_allocated_bytes, &last_allocated_bytes));
  }
}

TEST(DexFileTest, create_using_file) {
  TemporaryFile tf;
  ASSERT_TRUE(tf.fd != -1);