Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure pointers indirected from Memory and pointing into Memory should retain originating object #1326

Conversation

matthiasblaesing
Copy link
Member

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.

@matthiasblaesing
Copy link
Member Author

@dbwiddis could you please have a look and give it a try/get it tested? This replaces #1325, at least it should work and fix more than the single observed case.

src/com/sun/jna/Memory.java Outdated Show resolved Hide resolved
@dblock
Copy link
Member

dblock commented Mar 13, 2021

This is A+, very nice work @matthiasblaesing. I was following the discussion out of curiosity, and thinking there has to be a better way to do this, and this is it.

Copy link
Contributor

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this solution. A few suggestions inline.

src/com/sun/jna/Memory.java Show resolved Hide resolved
src/com/sun/jna/Memory.java Outdated Show resolved Hide resolved
src/com/sun/jna/Memory.java Outdated Show resolved Hide resolved
@dbwiddis
Copy link
Contributor

dbwiddis commented Mar 14, 2021

Testing: Set up an Azure VM (Windows Server 2012 R2) and created a test method exercising the current (Advapi32Util.getTokenAccount) code in a loop, running as admin and collecting usernames/domains of the user of every running process. Ran with very small heap (2500K) to maintain pressure on the GC. OpenJDK 15.0.2.

@matthiasblaesing
Copy link
Member Author

@dbwiddis @dblock thank you both for review. I pushed an update, that should cover the points raised (I plan to partially squash the commits prior to merging, but I want to preserve review history until done)

@dbwiddis
Copy link
Contributor

LGTM!

@dblock
Copy link
Member

dblock commented Mar 14, 2021

Looks great, merge at will.

…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.
@matthiasblaesing matthiasblaesing merged commit 0de51b3 into java-native-access:master Mar 14, 2021
@matthiasblaesing matthiasblaesing deleted the share_internal_memory branch August 10, 2021 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants