[foreign-memaccess+abi] RFR: 8263018: Improve API for lifecycle of native resources [v10]

[Maurizio Cimadamore]

On Tue, 16 Mar 2021 11:11:45 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>>> > Less sure about `SegmentAllocator.of(scope)` -> `ResourceScope.toAllocator()`, FWIW I noticed this is a subset of `malloc`:
>>> > ResourceScope scope = ...
>>> > SegmentAllocator fixedScopeAllocator = SegmentAllocator.malloc(() -> scope);
>>> > There might also be some interconnections with `ScopedSegmentAllocator`? Seems like we either want to compose through inheritance or have some bi-direction between the two abstractions.
>>> 
>>> Good point - if we had a ScopedSegmentAllocator abstraction, I believe creating an allocator from a scope would not be as important (if clients need both an allocator and a scope, well they can just allocate a ScopedSegmentAllocator).
>> 
>> I made an attempt at this - captured in the following javadoc:
>> 
>> http://cr.openjdk.java.net/~mcimadamore/panama/resourceScope-javadoc_v6_exp/
>> 
>> There is a new ScopedMemoryAllocator - and some of the existing factory now return that. Note that now the difference between SegmentAllocator.malloc and SegmentAllocator.scoped is cleared, as only the latter gives you a ScopedSegmentAllocator.
>> 
>> All the arena allocators are also ScopedSegmentAllocators.
>> 
>> The recycling allocator has been moved off to the side in MemorySegment::toAllocator.
>> 
>> Is this an improvement? While more verbose than the old native scope, I think there's a lot more composition going on here, as now users can select full spectrum of { shared, confined } x { bounded vs. unbounded allocation } x { explicit vs. implicit close } - the former NativeScope used to be a point (although common) in this space: confined x { bounded vs. unbounded allocation } x explicit close.
>
> Small observation: I realized that _all_ segment allocators can be associated with a scope. For allocators in which the scope is not important, we can just return the global scope.
> 
> In other words, associating a scope to an allocator is mostly harmless - either the allocator does have a lifecycle (and that is managed by the scope), or the allocator does not have one (in which case a neutral scope like global scope can be used).
> 
> This might address the problem of having a single entity which is connected to both an allocator and a scope - but it doesn't solve the usability problem of the try-with-resources. Unless we make SegmentAlocator AutoCloseable. But, if we make SegmentAllocator auto-closeable on the basis that an allocator has-a scope - then, on that basis it seems that even MemorySegment, VaList, ... should join for the ride? This seems like a slippery slope.

To bring this discussion closer to actual use cases - the thing that worries me is examples like these:

          try (var url = toCString(urlStr)) {
               curl_easy_setopt(curl, CURLOPT_URL(), url.address());
               int res = curl_easy_perform(curl);
               if (res != CURLE_OK()) {
                   String error = toJavaStringRestricted(curl_easy_strerror(res));
                   System.out.println("Curl error: " + error);
                   curl_easy_cleanup(curl);
               }
           }
(this can be found in the list of jextract samples, in [1]).

This code just needs a native string (there is a similar case in the clang implementation used by jextract). Now, there are many ways to rewrite that code (for simplicity I'll just focus on the first two lines):

1. remove the try with resource - and just use "default" scope (which is managed by the GC):

var url = toCString(urlStr);
curl_easy_setopt(curl, CURLOPT_URL(), url.address());
But there's a sneaky problem here: the function uses the address projection of the string segment - meaning that, at least in principle, the string segment can become unreachable (and hence freed) _before_ the native function is executed. This seems bad - and to avoid that, explicit resource deallocation seems preferrable (unless we want to start playing with reachability fences).

2. Use a native scope - after all this is jextracted code:

try (NativeScope scope = NativeScope.unboundedScope())) {
    var url = toCString(urlStr, scope);
    curl_easy_setopt(curl, CURLOPT_URL(), url.address());
    ...
}
This works as expected. There are two issues here: non-jextracted code cannot do this (as there's no NativeScope in the API); and, arguably, using an unbounded native scope just to allocate a single string seems overkill. That will probably allocate more memory than required, to name one.

3. Use ResourceScope + SegmentAllocator

try (ResourceScope scope = ResourceScope.ofConfined()) {
    var url = toCString(urlStr, SegmentAllocator.scoped(scope));
    curl_easy_setopt(curl, CURLOPT_URL(), url.address());
    ...
}
This works, and has the same semantics as the code that it's replacing.

Now maybe (3) is the best we can do - but I can't help but feeling that code like this will be common (e.g. converting a string on the fly and passing it to some native library) and having to create a scope _and_ an allocator to be able to do that (so that the right semantics occurs) seems unfortunate.

[1] - https://github.com/openjdk/panama-foreign/blob/foreign-jextract/doc/panama_jextract.md

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

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