Revert "MemoryIntArray: track the owned file descriptor in a PFD."

The reverted change causes a regression where we can get an IllegalStateException
during finalization as we are adopting the native fd in a ParcelFileDescriptor
which takes ownership of the fd. However, the order of finalization is undefined
and if the ParcelFileDescriptor is finalized before the MemoryIntArray we would
get an exception when running the finalization of the latter.

bug:124056170

This reverts commit c81f53f7f1.

Change-Id: I8debb9c5f4c87b1a657084139b27f40b7956fe59

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));
-        mMemoryAddr = nativeOpen(mFd.getFd(), mIsOwner);
+        mFd = nativeCreate(name, size);
+        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());
-        mMemoryAddr = nativeOpen(mFd.getFd(), mIsOwner);
+        mFd = pfd.detachFd();
+        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);
-            mFd.close();
-            mFd = null;
+            nativeClose(mFd, mMemoryAddr, mIsOwner);
+            mFd = -1;
             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
-        parcel.writeParcelable(mFd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
+        ParcelFileDescriptor pfd = ParcelFileDescriptor.adoptFd(mFd);
+        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;
         }
-
-        return false;
+        MemoryIntArray other = (MemoryIntArray) obj;
+        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.
-                Field fdField = MemoryIntArray.class.getDeclaredField("mFd");
-                fdField.setAccessible(true);
-                int fd = ((ParcelFileDescriptor)fdField.get(array)).getFd();
-                assertTrue("fd must be valid", fd != -1);
+                // Create the fd to stuff in the MemoryIntArray
+                final int fd = nativeCreateAshmem("foo", 1);
+
+                // Replace the fd with our ahsmem region
+                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);
 }