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

[Maurizio Cimadamore]

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

>> 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.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.
> 
> We could add an overload in `toCString` to also accept a `ResourceScope` (in which case the scoped allocator will be used) - this will simplify the code a bit:
> 
> try (ResourceScope scope = ResourceScope.ofConfined()) {
>     var url = toCString(urlStr, scope);
>     curl_easy_setopt(curl, CURLOPT_URL(), url.address());
>     ...
> }
> 
> But that also means that each API point accepting a SegmentAllocator will likely need a similar overload.
> 
> 
> [1] - https://github.com/openjdk/panama-foreign/blob/foreign-jextract/doc/panama_jextract.md

I've updated the branch. The main change is that now methods in CLinker have an overload accepting a ResourceScope - e.g. whenever we took an allocator, there is also a corresponding overload that takes only a scope. This way the user has a nice and gentle progression through the API complexity:

1. no scopes - everything is GC-managed (a la byte buffer)
2. deterministic closing with scopes - user creates a bunch of ResourceScope and all memory allocation is associated with such scopes
3. custom allocators - the user is not happy with default allocation mechanism - so, in addition to create a scope, a segment allocator will also be created.

While (1) and (2) will cover most of the "one-liner" examples, more realistic tests, such as those found in jextract will probably go for (3) [as they do today].

>From here, we have a couple of API extensions that we can evaluate and add later (they can be added in a compatible fashion):

a. Make all entities that have a scope should also have a close() method- this means that, instead of scope().close(), users can say close(), and use a TWR
b. consider if the above entities should also be AutoCloseable
c. Consider whether all segment allocators should be associated with a scope. We could add a default method returning ResourceScope.globalScope so that compatibility is not affected.
d. if we do (c) maybe we could add more factories overloads e.g. an arenaBounded which takes no scope, but creates the scope in a single shot.

Note that in the latest change I've converted StdLib test not to use NativeScope. I think the result is uniform and pleasing, and I didn't find myself missing it:

https://github.com/openjdk/panama-foreign/blob/d02e86f9fad8ad4ed17d84485e9fd198aa9eea73/test/jdk/java/foreign/StdLibTest.java

I've also added some ResourceScope overloads to jextract:

https://github.com/mcimadamore/panama-foreign/tree/jextract%2BresourceScope

And tried to convert my pet OpenGL application not to use NativeScope and again, it's all pretty readable.

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

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