Merge "Closes NotificationRankingUpdate fd after write" into main

2 files changed, 15 insertions, 12 deletions

diff --git a/core/java/android/service/notification/NotificationRankingUpdate.java b/core/java/android/service/notification/NotificationRankingUpdate.java
index 2a4cbaf79a75..46ea158b8f90 100644
--- a/core/java/android/service/notification/NotificationRankingUpdate.java
+++ b/core/java/android/service/notification/NotificationRankingUpdate.java
@@ -15,7 +15,6 @@
  */
 package android.service.notification;
 
-import android.annotation.Nullable;
 import android.annotation.SuppressLint;
 import android.app.Notification;
 import android.os.Bundle;
@@ -26,6 +25,7 @@ import android.system.ErrnoException;
 import android.system.OsConstants;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import java.nio.ByteBuffer;
@@ -75,10 +75,6 @@ public class NotificationRankingUpdate implements Parcelable {
                 }
                 // We only need read-only access to the shared memory region.
                 buffer = mRankingMapFd.mapReadOnly();
-                if (buffer == null) {
-                    mRankingMap = null;
-                    return;
-                }
                 byte[] payload = new byte[buffer.remaining()];
                 buffer.get(payload);
                 mapParcel.unmarshall(payload, 0, payload.length);
@@ -98,7 +94,7 @@ public class NotificationRankingUpdate implements Parcelable {
             } finally {
                 mapParcel.recycle();
                 if (buffer != null && mRankingMapFd != null) {
-                    mRankingMapFd.unmap(buffer);
+                    SharedMemory.unmap(buffer);
                     mRankingMapFd.close();
                 }
             }
@@ -210,6 +206,7 @@ public class NotificationRankingUpdate implements Parcelable {
                                     new NotificationListenerService.Ranking[0]
                             )
                     );
+            ByteBuffer buffer = null;
 
             try {
                 // Parcels the ranking map and measures its size.
@@ -217,13 +214,10 @@ public class NotificationRankingUpdate implements Parcelable {
                 int mapSize = mapParcel.dataSize();
 
                 // Creates a new SharedMemory object with enough space to hold the ranking map.
-                SharedMemory mRankingMapFd = SharedMemory.create(mSharedMemoryName, mapSize);
-                if (mRankingMapFd == null) {
-                    return;
-                }
+                mRankingMapFd = SharedMemory.create(mSharedMemoryName, mapSize);
 
                 // Gets a read/write buffer mapping the entire shared memory region.
-                final ByteBuffer buffer = mRankingMapFd.mapReadWrite();
+                buffer = mRankingMapFd.mapReadWrite();
                 // Puts the ranking map into the shared memory region buffer.
                 buffer.put(mapParcel.marshall(), 0, mapSize);
                 // Protects the region from being written to, by setting it to be read-only.
@@ -238,6 +232,12 @@ public class NotificationRankingUpdate implements Parcelable {
                 throw new RuntimeException(e);
             } finally {
                 mapParcel.recycle();
+                // To prevent memory leaks, we can close the ranking map fd here.
+                // Because a reference to this still exists
+                if (buffer != null && mRankingMapFd != null) {
+                    SharedMemory.unmap(buffer);
+                    mRankingMapFd.close();
+                }
             }
         } else {
             out.writeParcelable(mRankingMap, flags);
@@ -247,7 +247,7 @@ public class NotificationRankingUpdate implements Parcelable {
     /**
      * @hide
      */
-    public static final @android.annotation.NonNull Parcelable.Creator<NotificationRankingUpdate> CREATOR
+    public static final @NonNull Parcelable.Creator<NotificationRankingUpdate> CREATOR
             = new Parcelable.Creator<NotificationRankingUpdate>() {
         public NotificationRankingUpdate createFromParcel(Parcel parcel) {
             return new NotificationRankingUpdate(parcel);
diff --git a/core/tests/coretests/src/android/service/notification/NotificationRankingUpdateTest.java b/core/tests/coretests/src/android/service/notification/NotificationRankingUpdateTest.java
index 0855268411eb..1bdb006c3465 100644
--- a/core/tests/coretests/src/android/service/notification/NotificationRankingUpdateTest.java
+++ b/core/tests/coretests/src/android/service/notification/NotificationRankingUpdateTest.java
@@ -472,6 +472,9 @@ public class NotificationRankingUpdateTest {
         NotificationRankingUpdate nru = generateUpdate(getContext());
         Parcel parcel = Parcel.obtain();
         nru.writeToParcel(parcel, 0);
+        if (Flags.rankingUpdateAshmem()) {
+            assertTrue(nru.isFdNotNullAndClosed());
+        }
         parcel.setDataPosition(0);
         NotificationRankingUpdate nru1 = NotificationRankingUpdate.CREATOR.createFromParcel(parcel);
         // The rankingUpdate file descriptor is only non-null in the new path.