[foreign-memaccess] RFR: 8253243: Investigate ways to make MemorySegment::ofNativeRestricted more composable [v2]

Maurizio Cimadamore mcimadamore at openjdk.java.net
Wed Sep 16 16:17:30 UTC 2020


> This patch simplifies the MemorySegment::ofNativeRestricted API, based on a number of observations:
> 
> * now that we have shared segment support, passing an explicit owner thread isn't that useful (e.g. this is no longer an
>   escape hatch, and can - and should - be done with public, safe API)
> * if we had a public safe API to add cleanup action to existing segments, we could also get rid of the cleanup parameter
> * if a custom cleanup action embedded some other object, that object will be kept reachable, so no attachment API is
>   needed
> * if we moved the method to MemoryAddress, we would streamline uses of this API quite a bit, as in
>   `address.asSegmentRestricted(100)` - e.g. only size is needed
> 
> Adding the new API for attaching cleanup action is relatively straightforward - as usual we have to dup the segment and
> the scope, and create a new segment, with a new scope whose cleanup action calls the custom runnable before the
> original action (that original action should still be called, since otherwise we would fail to free/unmap memory). Any
> exception thrown by the cleanup action is caught and ignored, so as not to mess with the critical cleanup which has to
> occur.  A nice consequence of this work is that the surface for unsafety is much reduced, and the various methods
> compose much better. For instance, the "everything" segment can be expressed as follows:  MemoryAddress.NULL
>     .asSegmentRestricted(Long.MAX_VALUE)
>     .withOwnerThread(null)
>     .withAccessModes(READ | WRITE);
> 
> Which is, I think, much nicer. The changes in tests also suggest that the new API is easier on clients, and in one case
> (TestCleaner) the need for opting into unsafe operation actually vanished.
> I also did a bit of cleaning on the javadoc, which appeared to be off-sync when it comes to confinement.

Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:

  Address review comments

-------------

Changes:
  - all: https://git.openjdk.java.net/panama-foreign/pull/328/files
  - new: https://git.openjdk.java.net/panama-foreign/pull/328/files/3a186d46..d810c2a1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=panama-foreign&pr=328&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=panama-foreign&pr=328&range=00-01

  Stats: 8 lines in 2 files changed: 6 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/panama-foreign/pull/328.diff
  Fetch: git fetch https://git.openjdk.java.net/panama-foreign pull/328/head:pull/328

PR: https://git.openjdk.java.net/panama-foreign/pull/328


More information about the panama-dev mailing list