Skip to content

Commit

Permalink
Ensure pointers indirected from Memory and pointing into Memory shoul…
Browse files Browse the repository at this point in the history
…d retain 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.
  • Loading branch information
matthiasblaesing committed Mar 13, 2021
1 parent 7bf69fc commit 0f2a5e6
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 2 deletions.
37 changes: 36 additions & 1 deletion src/com/sun/jna/Memory.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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));
}

/**
Expand Down Expand Up @@ -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;
}
}
}
44 changes: 43 additions & 1 deletion test/com/sun/jna/MemoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 0f2a5e6

Please sign in to comment.