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

Commit a010ddbb authored by Christopher Ferris's avatar Christopher Ferris Committed by Gerrit Code Review
Browse files

Merge "Fix memory leak of DexFile handle after release"

parents 447a1ead 489c3a8b
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);