diff --git a/core/java/android/os/MemoryFile.java b/core/java/android/os/MemoryFile.java index 2e4d9b1ade02b..65e83c77957a3 100644 --- a/core/java/android/os/MemoryFile.java +++ b/core/java/android/os/MemoryFile.java @@ -18,6 +18,7 @@ package android.os; import android.util.Log; +import java.io.FileDescriptor; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -35,19 +36,19 @@ import java.io.OutputStream; public class MemoryFile { private static String TAG = "MemoryFile"; - - // returns fd - private static native int native_open(String name, int length) throws IOException; - // returns memory address for ashmem region - private static native int native_mmap(int fd, int length) throws IOException; - private static native void native_close(int fd); - private static native int native_read(int fd, int address, byte[] buffer, - int srcOffset, int destOffset, int count, boolean isUnpinned) throws IOException; - private static native void native_write(int fd, int address, byte[] buffer, - int srcOffset, int destOffset, int count, boolean isUnpinned) throws IOException; - private static native void native_pin(int fd, boolean pin) throws IOException; - private int mFD; // ashmem file descriptor + private static native FileDescriptor native_open(String name, int length) throws IOException; + // returns memory address for ashmem region + private static native int native_mmap(FileDescriptor fd, int length) throws IOException; + private static native void native_munmap(int addr, int length) throws IOException; + private static native void native_close(FileDescriptor fd); + private static native int native_read(FileDescriptor fd, int address, byte[] buffer, + int srcOffset, int destOffset, int count, boolean isUnpinned) throws IOException; + private static native void native_write(FileDescriptor fd, int address, byte[] buffer, + int srcOffset, int destOffset, int count, boolean isUnpinned) throws IOException; + private static native void native_pin(FileDescriptor fd, boolean pin) throws IOException; + + private FileDescriptor mFD; // ashmem file descriptor private int mAddress; // address of ashmem memory private int mLength; // total length of our ashmem region private boolean mAllowPurging = false; // true if our ashmem region is unpinned @@ -69,15 +70,40 @@ public class MemoryFile * Closes and releases all resources for the memory file. */ public void close() { - if (mFD > 0) { + deactivate(); + if (!isClosed()) { native_close(mFD); - mFD = 0; } } + private void deactivate() { + if (!isDeactivated()) { + try { + native_munmap(mAddress, mLength); + mAddress = 0; + } catch (IOException ex) { + Log.e(TAG, ex.toString()); + } + } + } + + /** + * Checks whether the memory file has been deactivated. + */ + private boolean isDeactivated() { + return mAddress == 0; + } + + /** + * Checks whether the memory file has been closed. + */ + private boolean isClosed() { + return !mFD.valid(); + } + @Override protected void finalize() { - if (mFD > 0) { + if (!isClosed()) { Log.e(TAG, "MemoryFile.finalize() called while ashmem still open"); close(); } @@ -132,7 +158,6 @@ public class MemoryFile @return OutputStream */ public OutputStream getOutputStream() { - return new MemoryOutputStream(); } @@ -145,9 +170,13 @@ public class MemoryFile * @param destOffset offset into the byte array buffer to read into. * @param count number of bytes to read. * @return number of bytes read. + * @throws IOException if the memory file has been purged or deactivated. */ public int readBytes(byte[] buffer, int srcOffset, int destOffset, int count) throws IOException { + if (isDeactivated()) { + throw new IOException("Can't read from deactivated memory file."); + } if (destOffset < 0 || destOffset > buffer.length || count < 0 || count > buffer.length - destOffset || srcOffset < 0 || srcOffset > mLength @@ -165,9 +194,13 @@ public class MemoryFile * @param srcOffset offset into the byte array buffer to write from. * @param destOffset offset into the memory file to write to. * @param count number of bytes to write. + * @throws IOException if the memory file has been purged or deactivated. */ public void writeBytes(byte[] buffer, int srcOffset, int destOffset, int count) throws IOException { + if (isDeactivated()) { + throw new IOException("Can't write to deactivated memory file."); + } if (srcOffset < 0 || srcOffset > buffer.length || count < 0 || count > buffer.length - srcOffset || destOffset < 0 || destOffset > mLength diff --git a/core/jni/android_os_MemoryFile.cpp b/core/jni/android_os_MemoryFile.cpp index edf7dc45991b9..9450e158dee55 100644 --- a/core/jni/android_os_MemoryFile.cpp +++ b/core/jni/android_os_MemoryFile.cpp @@ -26,7 +26,7 @@ namespace android { -static jint android_os_MemoryFile_open(JNIEnv* env, jobject clazz, jstring name, jint length) +static jobject android_os_MemoryFile_open(JNIEnv* env, jobject clazz, jstring name, jint length) { const char* namestr = (name ? env->GetStringUTFChars(name, NULL) : NULL); @@ -37,28 +37,45 @@ static jint android_os_MemoryFile_open(JNIEnv* env, jobject clazz, jstring name, if (name) env->ReleaseStringUTFChars(name, namestr); - if (result < 0) + if (result < 0) { jniThrowException(env, "java/io/IOException", "ashmem_create_region failed"); - return result; + return NULL; + } + + return jniCreateFileDescriptor(env, result); } -static jint android_os_MemoryFile_mmap(JNIEnv* env, jobject clazz, jint fd, jint length) +static jint android_os_MemoryFile_mmap(JNIEnv* env, jobject clazz, jobject fileDescriptor, + jint length) { + int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); jint result = (jint)mmap(NULL, length, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); if (!result) jniThrowException(env, "java/io/IOException", "mmap failed"); return result; } -static void android_os_MemoryFile_close(JNIEnv* env, jobject clazz, jint fd) +static void android_os_MemoryFile_munmap(JNIEnv* env, jobject clazz, jint addr, jint length) { - close(fd); + int result = munmap((void *)addr, length); + if (result < 0) + jniThrowException(env, "java/io/IOException", "munmap failed"); +} + +static void android_os_MemoryFile_close(JNIEnv* env, jobject clazz, jobject fileDescriptor) +{ + int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); + if (fd >= 0) { + jniSetFileDescriptorOfFD(env, fileDescriptor, -1); + close(fd); + } } static jint android_os_MemoryFile_read(JNIEnv* env, jobject clazz, - jint fd, jint address, jbyteArray buffer, jint srcOffset, jint destOffset, + jobject fileDescriptor, jint address, jbyteArray buffer, jint srcOffset, jint destOffset, jint count, jboolean unpinned) { + int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); if (unpinned && ashmem_pin_region(fd, 0, 0) == ASHMEM_WAS_PURGED) { ashmem_unpin_region(fd, 0, 0); jniThrowException(env, "java/io/IOException", "ashmem region was purged"); @@ -76,9 +93,10 @@ static jint android_os_MemoryFile_read(JNIEnv* env, jobject clazz, } static jint android_os_MemoryFile_write(JNIEnv* env, jobject clazz, - jint fd, jint address, jbyteArray buffer, jint srcOffset, jint destOffset, + jobject fileDescriptor, jint address, jbyteArray buffer, jint srcOffset, jint destOffset, jint count, jboolean unpinned) { + int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); if (unpinned && ashmem_pin_region(fd, 0, 0) == ASHMEM_WAS_PURGED) { ashmem_unpin_region(fd, 0, 0); jniThrowException(env, "java/io/IOException", "ashmem region was purged"); @@ -95,8 +113,9 @@ static jint android_os_MemoryFile_write(JNIEnv* env, jobject clazz, return count; } -static void android_os_MemoryFile_pin(JNIEnv* env, jobject clazz, jint fd, jboolean pin) +static void android_os_MemoryFile_pin(JNIEnv* env, jobject clazz, jobject fileDescriptor, jboolean pin) { + int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); int result = (pin ? ashmem_pin_region(fd, 0, 0) : ashmem_unpin_region(fd, 0, 0)); if (result < 0) { jniThrowException(env, "java/io/IOException", NULL); @@ -104,12 +123,13 @@ static void android_os_MemoryFile_pin(JNIEnv* env, jobject clazz, jint fd, jbool } static const JNINativeMethod methods[] = { - {"native_open", "(Ljava/lang/String;I)I", (void*)android_os_MemoryFile_open}, - {"native_mmap", "(II)I", (void*)android_os_MemoryFile_mmap}, - {"native_close", "(I)V", (void*)android_os_MemoryFile_close}, - {"native_read", "(II[BIIIZ)I", (void*)android_os_MemoryFile_read}, - {"native_write", "(II[BIIIZ)V", (void*)android_os_MemoryFile_write}, - {"native_pin", "(IZ)V", (void*)android_os_MemoryFile_pin}, + {"native_open", "(Ljava/lang/String;I)Ljava/io/FileDescriptor;", (void*)android_os_MemoryFile_open}, + {"native_mmap", "(Ljava/io/FileDescriptor;I)I", (void*)android_os_MemoryFile_mmap}, + {"native_munmap", "(II)V", (void*)android_os_MemoryFile_munmap}, + {"native_close", "(Ljava/io/FileDescriptor;)V", (void*)android_os_MemoryFile_close}, + {"native_read", "(Ljava/io/FileDescriptor;I[BIIIZ)I", (void*)android_os_MemoryFile_read}, + {"native_write", "(Ljava/io/FileDescriptor;I[BIIIZ)V", (void*)android_os_MemoryFile_write}, + {"native_pin", "(Ljava/io/FileDescriptor;Z)V", (void*)android_os_MemoryFile_pin}, }; static const char* const kClassPathName = "android/os/MemoryFile"; diff --git a/tests/AndroidTests/src/com/android/unit_tests/os/MemoryFileTest.java b/tests/AndroidTests/src/com/android/unit_tests/os/MemoryFileTest.java index 508afcf411b81..5161f7beda0e4 100644 --- a/tests/AndroidTests/src/com/android/unit_tests/os/MemoryFileTest.java +++ b/tests/AndroidTests/src/com/android/unit_tests/os/MemoryFileTest.java @@ -17,16 +17,17 @@ package com.android.unit_tests.os; import android.os.MemoryFile; +import android.test.suitebuilder.annotation.LargeTest; import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.SmallTest; -import junit.framework.TestCase; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.IOException; - -import java.util.List; import java.util.ArrayList; +import java.util.List; + +import junit.framework.TestCase; public class MemoryFileTest extends TestCase { @@ -94,6 +95,74 @@ public class MemoryFileTest extends TestCase { file.close(); } + // Tests that close() is idempotent + @SmallTest + public void testCloseClose() throws Exception { + MemoryFile file = new MemoryFile("MemoryFileTest", 1000000); + byte[] data = new byte[512]; + file.writeBytes(data, 0, 0, 128); + file.close(); + file.close(); + } + + // Tests that we can't read from a closed memory file + @SmallTest + public void testCloseRead() throws Exception { + MemoryFile file = new MemoryFile("MemoryFileTest", 1000000); + file.close(); + + try { + byte[] data = new byte[512]; + assertEquals(128, file.readBytes(data, 0, 0, 128)); + fail("readBytes() after close() did not throw IOException."); + } catch (IOException e) { + // this is what should happen + } + } + + // Tests that we can't write to a closed memory file + @SmallTest + public void testCloseWrite() throws Exception { + MemoryFile file = new MemoryFile("MemoryFileTest", 1000000); + file.close(); + + try { + byte[] data = new byte[512]; + file.writeBytes(data, 0, 0, 128); + fail("writeBytes() after close() did not throw IOException."); + } catch (IOException e) { + // this is what should happen + } + } + + // Tests that we can't call allowPurging() after close() + @SmallTest + public void testCloseAllowPurging() throws Exception { + MemoryFile file = new MemoryFile("MemoryFileTest", 1000000); + byte[] data = new byte[512]; + file.writeBytes(data, 0, 0, 128); + file.close(); + + try { + file.allowPurging(true); + fail("allowPurging() after close() did not throw IOException."); + } catch (IOException e) { + // this is what should happen + } + } + + // Tests that we don't leak file descriptors or mmap areas + @LargeTest + public void testCloseLeak() throws Exception { + // open enough memory files that we should run out of + // file descriptors or address space if we leak either. + for (int i = 0; i < 1025; i++) { + MemoryFile file = new MemoryFile("MemoryFileTest", 5000000); + file.writeBytes(testString, 0, 0, testString.length); + file.close(); + } + } + private static final byte[] testString = new byte[] { 3, 1, 4, 1, 5, 9, 2, 6, 5, 3, 5, 8, 9, 7, 9, 3, 2, 3, 8, 4, 6, 2, 6, 4, 3, 3, 8, 3, 2, 7, 9, 5, 0, 2, 8, 8, 4, 1, 9, 7, 1, 6, 9, 3, 9, 9, 3, 7, 5, 1, 0, 5, 8, 2, 0, 9, 7, 4, 9, 4, 4, 5, 9, 2, 3, 0, 7, 8, 1, 6, 4, 0, 6, 2, 8, 6, 2, 0, 8, 9, 9, 8, 6, 2, 8, 0, 3, 4, 8, 2, 5, 3, 4, 2, 1, 1, 7, 0, 6, 7, 9, 8, 2, 1, 4, 8, 0, 8, 6, 5, 1, 3, 2, 8, 2, 3, 0, 6, 6, 4, 7, 0, 9, 3, 8, 4, 4, 6, 0, 9, 5, 5, 0, 5, 8, 2, 2, 3, 1, 7, 2,