Fix internal issue #1898791 ("PlatformAddressFactory PlatformAddress cache

causes memory to not be freed in a timely manner.").

The change makes the two PlatformAddressFactory.alloc() methods always
allocate separate PlatformAddress objects instead of looking for them
in the cache. This means that, should they have auto-free turned on,
there won't be a reference in the cache that prevents the freeing from
happening. And since we're talking about freshly-allocated memory anyway,
it would be surprising that either a matching address would be found in
cache *or* that a subsequent call to on() would ever happen with the
same address. So, it's a win on several fronts.

1 files changed, 34 insertions, 4 deletions

diff --git a/luni/src/main/java/org/apache/harmony/luni/platform/PlatformAddressFactory.java b/luni/src/main/java/org/apache/harmony/luni/platform/PlatformAddressFactory.java
index 9ec0fcd..9ac8064 100644
--- a/luni/src/main/java/org/apache/harmony/luni/platform/PlatformAddressFactory.java
+++ b/luni/src/main/java/org/apache/harmony/luni/platform/PlatformAddressFactory.java
@@ -44,7 +44,8 @@ public class PlatformAddressFactory {
     private final static int MAX_PROBES = 5;
 
     /**
-     * A cycling index (0 to MAX_PROBES-1) used to replace elements in the cache.
+     * A cycling index (0 to MAX_PROBES-1) used to replace elements in
+     * the cache.
      */
     private static int replacementIndex = 0;
 
@@ -52,8 +53,24 @@ public class PlatformAddressFactory {
      * Array of PlatformAddress references kept from garbage collection.
      */
     private static PlatformAddress[] cache = new PlatformAddress[CACHE_SIZE];
-    // END android-added
 
+    /**
+     * Constructs a {@code PlatformAddress} or returns
+     * {@link PlatformAddress#NULL} if given a {@code null} address.
+     *
+     * @param address the start address for the memory; {@code 0} means
+     * {@code null}
+     * @param size the size of the memory in bytes
+     * @return an appropriately-constructed {@code PlatformAddress}
+     */
+    private static PlatformAddress make(int value, long size) {
+        if (value == 0) {
+            return PlatformAddress.NULL;
+        }
+
+        return new PlatformAddress(value, size);
+    }
+    // END android-added
 
     // BEGIN android-changed
     public synchronized static PlatformAddress on(int value, long size) {
@@ -101,7 +118,17 @@ public class PlatformAddressFactory {
      */
     public static PlatformAddress alloc(int size) {
         int osAddress = PlatformAddress.osMemory.malloc(size);
-        PlatformAddress newMemory = on(osAddress, size);
+        // BEGIN android-changed
+        /*
+         * We use make() and not on() here, for a couple reasons:
+         * First and foremost, doing so means that if the client uses
+         * address.autoFree() (to enable auto-free on gc) the cache
+         * won't prevent the freeing behavior. Second, this avoids
+         * polluting the cache with addresses that aren't likely to be
+         * reused anyway.
+         */
+        PlatformAddress newMemory = make(osAddress, size);
+        // END android-changed
         PlatformAddress.memorySpy.alloc(newMemory);
         return newMemory;
     }
@@ -117,7 +144,10 @@ public class PlatformAddressFactory {
     public static PlatformAddress alloc(int size, byte init) {
         int osAddress = PlatformAddress.osMemory.malloc(size);
         PlatformAddress.osMemory.memset(osAddress, init, size);
-        PlatformAddress newMemory = on(osAddress, size);
+        // BEGIN android-changed
+        // See above for the make() vs. on() rationale.
+        PlatformAddress newMemory = make(osAddress, size);
+        // END android-changed
         PlatformAddress.memorySpy.alloc(newMemory);
         return newMemory;
     }