Ensure pointers indirected from Memory and pointing into Memory retai…

…n originating object

The use case is this:

- native supplies the required size of a memory region
- a Memory object is created with the right memory size
- native fills the memory with a structure _and_ additional objects,
  that are held in that region, but are only referenced from the
  structure (one such example would be a Structure.ByReference)

As long as the resulting structure stays strongly referenced from Java,
all is good. When the toplevel structure goes ouf of scope, the memory
backing the strucure is not strongly referenced anymore and will be
freeed by GC.

This change fixes the issue by ensuring, that the pointers used in
substructures are SharedMemory objects, holding a strong references to
the original Memory object.

CHANGES.md

@@ -14,6 +14,7 @@ Bug Fixes
 ---------
 * [#1317](https://github.com/java-native-access/jna/pull/1317): Change the maven coordinates of the JPMS artifacts from classifier `jpms` to custom artifact ids `jna-jpms` and `jna-platform-jpms` - [@matthiasblaesing](https://github.com/matthiasblaesing).
 * [#1322](https://github.com/java-native-access/jna/pull/1322): Handle 0-length domain names in `c.s.j.p.win32.Advapi32Util#getAccountBySid` - [@dbwiddis](https://github.com/dbwiddis).
+* [#1326](https://github.com/java-native-access/jna/pull/1326): Ensure pointers indirected from Memory and pointing into Memory retain originating object - [@matthiasblaesing](https://github.com/matthiasblaesing).
 
 Important Changes
 -----------------

src/com/sun/jna/Memory.java

@@ -460,6 +460,20 @@ public void read(long bOff, double[] buf, int index, int length) {
         super.read(bOff, buf, index, length);
     }
 
+    /**
+     * Indirect the native pointer to <code>malloc</code> space, a la
+     * <code>Pointer.read</code>.  But this method performs a bounds checks to
+     * ensure that the indirection does not cause memory outside the
+     * <code>malloc</code>ed space to be accessed.
+     *
+     * @see Pointer#read(long,Pointer[],int,int)
+     */
+    @Override
+    public void read(long bOff, Pointer[] buf, int index, int length) {
+        boundsCheck(bOff, length * Native.POINTER_SIZE);
+        super.read(bOff, buf, index, length);
+    }
+
     //////////////////////////////////////////////////////////////////////////
     // Raw write methods
     //////////////////////////////////////////////////////////////////////////
@@ -675,7 +689,7 @@ public double getDouble(long offset) {
     @Override
     public Pointer getPointer(long offset) {
         boundsCheck(offset, Native.POINTER_SIZE);
-        return super.getPointer(offset);
+        return shareIfInternalPointer(super.getPointer(offset));
     }
 
     /**
@@ -862,4 +876,25 @@ protected static long malloc(long size) {
     public String dump() {
         return dump(0, (int)size());
     }
+
+    /**
+     * Check whether the supplied Pointer object points into the memory region
+     * backed by this memory object. The intention is to prevent premature GC
+     * of the Memory object.
+     *
+     * @param target Pointer to check
+     * @return {@code target} if target does not point into the region covered
+     * by this memory object, a newly {@code SharedMemory} object, if the pointer
+     * points to memory backed by this Memory object.
+     */
+    private Pointer shareIfInternalPointer(Pointer target) {
+        if(target == null) {
+            return null;
+        }
+        if (target.peer >= this.peer && (this.peer + this.size) > target.peer) {
+            return this.share(target.peer - this.peer);
+        } else {
+            return target;
+        }
+    }
 }

test/com/sun/jna/MemoryTest.java

@@ -23,11 +23,16 @@
  */
 package com.sun.jna;
 
+
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.nio.ByteBuffer;
-
 import junit.framework.TestCase;
+import org.junit.Assert;
+
+import static com.sun.jna.Native.POINTER_SIZE;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.not;
 
 public class MemoryTest extends TestCase {
 
@@ -199,6 +204,43 @@ public void testRemoveAllocatedMemory() {
         assertEquals(0, Memory.integrityCheck());
     }
 
+    public void testGetSharedMemory() {
+        Memory mem1 = new Memory(512);
+        Memory mem2 = new Memory(512);
+
+        // Get pointers into the two memory objects
+        Pointer stringStart1 = mem1.share(128);
+        Pointer stringStart2 = mem2.share(128);
+
+        mem1.setPointer(10 * POINTER_SIZE, stringStart1);
+        mem1.setPointer(11 * POINTER_SIZE, stringStart2);
+
+        // The pointer in mem1 at offset 10 * POINTER_SIZE points into the
+        // memory region of mem1, while the pointer at offset 11 * POINTER_SIZE
+        // points to the second memory region
+
+        // It is expected, that resolution of the first pointer results in
+        // an instance of SharedMemory (a subclass of Memory, that retains a
+        // reference on the originating Memory object)
+        Assert.assertThat(mem1.getPointer(10 * POINTER_SIZE), instanceOf(Pointer.class));
+        Assert.assertThat(mem1.getPointer(10 * POINTER_SIZE), instanceOf(Memory.class));
+        // The second pointer lies outside of memory 1, so it must not be a
+        // Memory object, but a raw pointer
+        Assert.assertThat(mem1.getPointer(11 * POINTER_SIZE), instanceOf(Pointer.class));
+        Assert.assertThat(mem1.getPointer(11 * POINTER_SIZE), not(instanceOf(Memory.class)));
+
+        // It is expected, that Memory#read called for pointers shows the same
+        // behaviour as direct calls to getPointer calls with the corresponding
+        // offsets
+        Pointer[] pointers = new Pointer[2];
+        mem1.read(10 * POINTER_SIZE, pointers, 0, 2);
+
+        Assert.assertThat(pointers[0], instanceOf(Pointer.class));
+        Assert.assertThat(pointers[0], instanceOf(Memory.class));
+        Assert.assertThat(pointers[1], instanceOf(Pointer.class));
+        Assert.assertThat(pointers[1], not(instanceOf(Memory.class)));
+    }
+
     public static void main(String[] args) {
         junit.textui.TestRunner.run(MemoryTest.class);
     }