From 5ceb7f6431f447aa9d4e949578ce5d4b90b8297c Mon Sep 17 00:00:00 2001 From: Deepanshu Gupta Date: Tue, 2 Dec 2014 12:41:23 +0530 Subject: [PATCH] Ensure that RandomAccessFile is not leaked. This change ensures that RandomAccessFile is always closed. Also, it changes the BufferIterator implementation to delegate to the byte buffer directly instead of mainitaining the position itself. Change-Id: I8f93e80c91a7c35e6a7fb015185b8b11e36b6286 --- .../libcore/io/BridgeBufferIterator.java | 43 +++++++------------ .../libcore/io/MemoryMappedFile_Delegate.java | 29 ++++++++----- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/libcore/io/BridgeBufferIterator.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/libcore/io/BridgeBufferIterator.java index 2075989e59196..7e361a11f3877 100644 --- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/libcore/io/BridgeBufferIterator.java +++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/libcore/io/BridgeBufferIterator.java @@ -16,74 +16,61 @@ package com.android.layoutlib.bridge.libcore.io; -import java.nio.MappedByteBuffer; +import java.nio.ByteBuffer; import libcore.io.BufferIterator; /** - * Provides an implementation of {@link BufferIterator} over a {@link MappedByteBuffer}. + * Provides an implementation of {@link BufferIterator} over a {@link ByteBuffer}. */ public class BridgeBufferIterator extends BufferIterator { - private int mPosition; private final long mSize; - private final MappedByteBuffer mMappedByteBuffer; + private final ByteBuffer mByteBuffer; - public BridgeBufferIterator(long size, MappedByteBuffer buffer) { + public BridgeBufferIterator(long size, ByteBuffer buffer) { mSize = size; - mMappedByteBuffer = buffer; + mByteBuffer = buffer; } @Override public void seek(int offset) { - assert offset < mSize; - mPosition = offset; + assert offset <= mSize; + mByteBuffer.position(offset); } @Override public void skip(int byteCount) { - assert mPosition + byteCount <= mSize; - mPosition += byteCount; + int newPosition = mByteBuffer.position() + byteCount; + assert newPosition <= mSize; + mByteBuffer.position(newPosition); } @Override public void readByteArray(byte[] dst, int dstOffset, int byteCount) { assert dst.length >= dstOffset + byteCount; - mMappedByteBuffer.position(mPosition); - mMappedByteBuffer.get(dst, dstOffset, byteCount); - mPosition = mMappedByteBuffer.position(); + mByteBuffer.get(dst, dstOffset, byteCount); } @Override public byte readByte() { - mMappedByteBuffer.position(mPosition); - byte b = mMappedByteBuffer.get(); - mPosition = mMappedByteBuffer.position(); - return b; + return mByteBuffer.get(); } @Override public int readInt() { - mMappedByteBuffer.position(mPosition); - int i = mMappedByteBuffer.getInt(); - mPosition = mMappedByteBuffer.position(); - return i; + return mByteBuffer.getInt(); } @Override public void readIntArray(int[] dst, int dstOffset, int intCount) { - mMappedByteBuffer.position(mPosition); while (--intCount >= 0) { - dst[dstOffset++] = mMappedByteBuffer.getInt(); + dst[dstOffset++] = mByteBuffer.getInt(); } - mPosition = mMappedByteBuffer.position(); } @Override public short readShort() { - mMappedByteBuffer.position(mPosition); - short s = mMappedByteBuffer.getShort(); - mPosition = mMappedByteBuffer.position(); - return s; + return mByteBuffer.getShort(); } } diff --git a/tools/layoutlib/bridge/src/libcore/io/MemoryMappedFile_Delegate.java b/tools/layoutlib/bridge/src/libcore/io/MemoryMappedFile_Delegate.java index 3c18bd3239d36..723d5c4b0d0cb 100644 --- a/tools/layoutlib/bridge/src/libcore/io/MemoryMappedFile_Delegate.java +++ b/tools/layoutlib/bridge/src/libcore/io/MemoryMappedFile_Delegate.java @@ -25,6 +25,7 @@ import android.system.ErrnoException; import java.io.File; import java.io.IOException; import java.io.RandomAccessFile; +import java.nio.ByteOrder; import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel.MapMode; import java.util.HashMap; @@ -59,15 +60,22 @@ public class MemoryMappedFile_Delegate { } path = path.substring(TARGET_PATH.length()); try { - RandomAccessFile file = new RandomAccessFile(new File(sRootPath, path), "r"); - long size = file.length(); - MemoryMappedFile_Delegate newDelegate = new MemoryMappedFile_Delegate(file); - long filePointer = file.getFilePointer(); - MemoryMappedFile mmFile = new MemoryMappedFile(filePointer, size); - long delegateIndex = sManager.addNewDelegate(newDelegate); - sMemoryMappedFileMap.put(mmFile, delegateIndex); - file.close(); // Also closes the channel opened by the delegate constructor. - return mmFile; + File f = new File(sRootPath, path); + if (!f.exists()) { + throw new ErrnoException("File not found: " + f.getPath(), 1); + } + RandomAccessFile file = new RandomAccessFile(f, "r"); + try { + long size = file.length(); + MemoryMappedFile_Delegate newDelegate = new MemoryMappedFile_Delegate(file); + long filePointer = file.getFilePointer(); + MemoryMappedFile mmFile = new MemoryMappedFile(filePointer, size); + long delegateIndex = sManager.addNewDelegate(newDelegate); + sMemoryMappedFileMap.put(mmFile, delegateIndex); + return mmFile; + } finally { + file.close(); + } } catch (IOException e) { throw new ErrnoException("mmapRO", 1, e); } @@ -85,7 +93,7 @@ public class MemoryMappedFile_Delegate { @LayoutlibDelegate static BufferIterator bigEndianIterator(MemoryMappedFile file) { MemoryMappedFile_Delegate delegate = getDelegate(file); - return new BridgeBufferIterator(delegate.mSize, delegate.mMappedByteBuffer); + return new BridgeBufferIterator(delegate.mSize, delegate.mMappedByteBuffer.duplicate()); } // TODO: implement littleEndianIterator() @@ -95,6 +103,7 @@ public class MemoryMappedFile_Delegate { // It's weird that map() takes size as long, but returns MappedByteBuffer which uses an int // to store the marker to the position. mMappedByteBuffer = file.getChannel().map(MapMode.READ_ONLY, 0, mSize); + assert mMappedByteBuffer.order() == ByteOrder.BIG_ENDIAN; } public static void setDataDir(File path) {