Merge "Revert "MemoryIntArray: track the owned file descriptor in a PFD."" into qt-dev

core/java/android/util/MemoryIntArray.java

@@ -20,9 +20,8 @@ import android.os.Parcel;
 import android.os.ParcelFileDescriptor;
 import android.os.Parcelable;
 
-import dalvik.system.CloseGuard;
-
 import libcore.io.IoUtils;
+import dalvik.system.CloseGuard;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -57,7 +56,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
 
     private final boolean mIsOwner;
     private final long mMemoryAddr;
-    private ParcelFileDescriptor mFd;
+    private int mFd = -1;
 
     /**
      * Creates a new instance.
@@ -72,8 +71,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
         }
         mIsOwner = true;
         final String name = UUID.randomUUID().toString();
-        mFd = ParcelFileDescriptor.adoptFd(nativeCreate(name, size));
+        mFd = nativeCreate(name, size);
-        mMemoryAddr = nativeOpen(mFd.getFd(), mIsOwner);
+        mMemoryAddr = nativeOpen(mFd, mIsOwner);
         mCloseGuard.open("close");
     }
 
@@ -83,8 +82,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
         if (pfd == null) {
             throw new IOException("No backing file descriptor");
         }
-        mFd = ParcelFileDescriptor.adoptFd(pfd.detachFd());
+        mFd = pfd.detachFd();
-        mMemoryAddr = nativeOpen(mFd.getFd(), mIsOwner);
+        mMemoryAddr = nativeOpen(mFd, mIsOwner);
         mCloseGuard.open("close");
     }
 
@@ -106,7 +105,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
     public int get(int index) throws IOException {
         enforceNotClosed();
         enforceValidIndex(index);
-        return nativeGet(mFd.getFd(), mMemoryAddr, index);
+        return nativeGet(mFd, mMemoryAddr, index);
     }
 
     /**
@@ -122,7 +121,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
         enforceNotClosed();
         enforceWritable();
         enforceValidIndex(index);
-        nativeSet(mFd.getFd(), mMemoryAddr, index, value);
+        nativeSet(mFd, mMemoryAddr, index, value);
     }
 
     /**
@@ -132,7 +131,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
      */
     public int size() throws IOException {
         enforceNotClosed();
-        return nativeSize(mFd.getFd());
+        return nativeSize(mFd);
     }
 
     /**
@@ -143,9 +142,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
     @Override
     public void close() throws IOException {
         if (!isClosed()) {
-            nativeClose(mFd.getFd(), mMemoryAddr, mIsOwner);
+            nativeClose(mFd, mMemoryAddr, mIsOwner);
-            mFd.close();
+            mFd = -1;
-            mFd = null;
             mCloseGuard.close();
         }
     }
@@ -154,7 +152,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
      * @return Whether this array is closed and shouldn't be used.
      */
     public boolean isClosed() {
-        return mFd == null;
+        return mFd == -1;
     }
 
     @Override
@@ -177,8 +175,13 @@ public final class MemoryIntArray implements Parcelable, Closeable {
 
     @Override
     public void writeToParcel(Parcel parcel, int flags) {
-        // Don't let writing to a parcel to close our fd - plz
+        ParcelFileDescriptor pfd = ParcelFileDescriptor.adoptFd(mFd);
-        parcel.writeParcelable(mFd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        try {
+            // Don't let writing to a parcel to close our fd - plz
+            parcel.writeParcelable(pfd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        } finally {
+            pfd.detachFd();
+        }
     }
 
     @Override
@@ -192,13 +195,13 @@ public final class MemoryIntArray implements Parcelable, Closeable {
         if (getClass() != obj.getClass()) {
             return false;
         }
-
+        MemoryIntArray other = (MemoryIntArray) obj;
-        return false;
+        return mFd == other.mFd;
     }
 
     @Override
     public int hashCode() {
-        return mFd.hashCode();
+        return mFd;
     }
 
     private void enforceNotClosed() {

core/tests/utiltests/Android.mk

@@ -18,6 +18,7 @@ LOCAL_STATIC_JAVA_LIBRARIES := \
     androidx.test.rules \
     frameworks-base-testutils \
     mockito-target-minus-junit4 \
+    androidx.test.ext.junit
 
 LOCAL_JAVA_LIBRARIES := android.test.runner android.test.base android.test.mock
 

core/tests/utiltests/jni/android_util_MemoryIntArrayTest.cpp

@@ -21,6 +21,36 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
+jint android_util_MemoryIntArrayTest_createAshmem(__attribute__((unused)) JNIEnv* env,
+        __attribute__((unused)) jobject clazz,
+        jstring name, jint size)
+{
+
+    if (name == NULL) {
+        return -1;
+    }
+
+    if (size < 0) {
+        return -1;
+    }
+
+    const char* nameStr = env->GetStringUTFChars(name, NULL);
+    const int ashmemSize = sizeof(std::atomic_int) * size;
+    int fd = ashmem_create_region(nameStr, ashmemSize);
+    env->ReleaseStringUTFChars(name, nameStr);
+
+    if (fd < 0) {
+        return -1;
+    }
+
+    int setProtResult = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE);
+    if (setProtResult < 0) {
+        return -1;
+    }
+
+    return fd;
+}
+
 void android_util_MemoryIntArrayTest_setAshmemSize(__attribute__((unused)) JNIEnv* env,
         __attribute__((unused)) jobject clazz, jint fd, jint size)
 {

core/tests/utiltests/jni/registration.cpp

@@ -16,14 +16,25 @@
 
 #include <jni.h>
 
+extern jint android_util_MemoryIntArrayTest_createAshmem(JNIEnv* env,
+        jobject clazz, jstring name, jint size);
 extern void android_util_MemoryIntArrayTest_setAshmemSize(JNIEnv* env,
        jobject clazz, jint fd, jint size);
 
 extern "C" {
+    JNIEXPORT jint JNICALL Java_android_util_MemoryIntArrayTest_nativeCreateAshmem(
+            JNIEnv * env, jobject obj, jstring name, jint size);
     JNIEXPORT void JNICALL Java_android_util_MemoryIntArrayTest_nativeSetAshmemSize(
             JNIEnv * env, jobject obj, jint fd, jint size);
 };
 
+JNIEXPORT jint JNICALL Java_android_util_MemoryIntArrayTest_nativeCreateAshmem(
+        __attribute__((unused)) JNIEnv * env,__attribute__((unused)) jobject obj,
+        jstring name, jint size)
+{
+    return android_util_MemoryIntArrayTest_createAshmem(env, obj, name, size);
+}
+
 JNIEXPORT void JNICALL Java_android_util_MemoryIntArrayTest_nativeSetAshmemSize(
         __attribute__((unused)) JNIEnv * env,__attribute__((unused)) jobject obj,
         jint fd, jint size)

core/tests/utiltests/src/android/util/MemoryIntArrayTest.java

@@ -23,9 +23,8 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import android.os.Parcel;
-import android.os.ParcelFileDescriptor;
 
-import androidx.test.runner.AndroidJUnit4;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
 
 import libcore.io.IoUtils;
 
@@ -36,6 +35,8 @@ import java.lang.reflect.Field;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
 @RunWith(AndroidJUnit4.class)
 public class MemoryIntArrayTest {
     static {
@@ -255,11 +256,13 @@ public class MemoryIntArrayTest {
                 // Create a MemoryIntArray to muck with
                 MemoryIntArray array = new MemoryIntArray(1);
 
-                // Grab the internal ashmem fd.
+                // Create the fd to stuff in the MemoryIntArray
-                Field fdField = MemoryIntArray.class.getDeclaredField("mFd");
+                final int fd = nativeCreateAshmem("foo", 1);
-                fdField.setAccessible(true);
+
-                int fd = ((ParcelFileDescriptor)fdField.get(array)).getFd();
+                // Replace the fd with our ahsmem region
-                assertTrue("fd must be valid", fd != -1);
+                Field fdFiled = MemoryIntArray.class.getDeclaredField("mFd");
+                fdFiled.setAccessible(true);
+                fdFiled.set(array, fd);
 
                 CountDownLatch countDownLatch = new CountDownLatch(2);
 
@@ -294,9 +297,10 @@ public class MemoryIntArrayTest {
         }
 
         if (!success) {
-            fail("MemoryIntArray should catch ashmem size changing under it");
+            fail("MemoryIntArray should catch ahshmem size changing under it");
         }
     }
 
+    private native int nativeCreateAshmem(String name, int size);
     private native void nativeSetAshmemSize(int fd, int size);
 }