[foreign-memaccess+abi] RFR: Misc javadoc fixes

Chris Hegarty chegar at openjdk.java.net
Thu Apr 22 18:01:39 UTC 2021


On Thu, 22 Apr 2021 14:30:17 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This short patch addresses some API review comments which have been raised by @jbf.
> 
> Following changes are made:
> 
> * addOnClose is renamed to addCloseAction, to remove ambiguity on when `add` happens
> * some comments about allocation confinement and thread safety have been moved in the arenaAllocator factory (from the top level javadoc)
> * The text in the thread-confinement section of `ResourceScope` has been massaged a little to describe how clients are expected to deal with issues when closing shared scopes in a more positive way (e.g. the new text suggests what to do "Add more synchronization" instead of saying "it's a bug"). The rewritten section also connects more nicely with what comes later (resource scope handles) which are all about avoiding close vs. access races.

Marked as reviewed by chegar (Committer).

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java line 111:

> 109:  *
> 110:  * <p>
> 111:  * Explicit shared resource scopes, while powerful, must be used with caution: if one or more threads accesses

The cautionary verbiage and qualification to "explicit shared" looks good.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java line 198:

> 196:      * @throws IllegalStateException if this scope has already been closed.
> 197:      */
> 198:     void addCloseAction(Runnable runnable);

The rename to addCloseAction is good, it better describes what the methods does and ties in nicely with "cleanup action".

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/SegmentAllocator.java line 358:

> 356:      * performed concurrently; conversely, if the arena allocator is associated with a <em>confined</em> resource scope,
> 357:      * allocation requests can only occur from the thread owning the allocator's resource scope.
> 358:      * <p>

LGTM.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/NativeMemorySegmentImpl.java line 126:

> 124:         if (cleanupAction != null) {
> 125:             scope.addCloseAction(cleanupAction);
> 126:         }

The test changes look fine.

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

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


More information about the panama-dev mailing list