am 761e0918: Unmap memory in MemoryFile.close().
Merge commit '761e0918d30b6a3f292625b44b86dffd1538bc78'
* commit '761e0918d30b6a3f292625b44b86dffd1538bc78':
Unmap memory in MemoryFile.close().
diff --git a/core/java/android/os/MemoryFile.java b/core/java/android/os/MemoryFile.java
index 2e4d9b1..65e83c7 100644
--- a/core/java/android/os/MemoryFile.java
+++ b/core/java/android/os/MemoryFile.java
@@ -18,6 +18,7 @@
import android.util.Log;
+import java.io.FileDescriptor;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -35,19 +36,19 @@
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 @@
* 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 @@
@return OutputStream
*/
public OutputStream getOutputStream() {
-
return new MemoryOutputStream();
}
@@ -145,9 +170,13 @@
* @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 @@
* @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 edf7dc4..9450e15 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 @@
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_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 @@
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 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 508afcf..5161f7b 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 @@
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,