[foreign-abi] RFR: 8248420: Add a variant of VaList::make which takes a NativeScope [v3]

[Paul Sandoz]

On Wed, 15 Jul 2020 20:06:26 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CSupport.java line 161:
>> 
>>> 160:          * of a va list does not need any native memory, so nothing has to be released. After calling {@code
>>> close()} on 161:          * such an instance {@link #isAlive()} will still return {@code true}.
>>> 162:          *
>> 
>> Is it possible hide the implementation detail so close behaviour is consistent across all implementations, even if on
>> some implementation no allocation is required? Especially since close also may close any native memory that is attached.
>
> I think it's possible by creating a dummy segment when doing the copy on Windows. The dummy segment can then be used to
> check if the VaList is alive or not, and we can also register it with the NativeScope so it gets closed by that. Feels
> kinda hacky though.  A proper solution is probably to create a Resource abstraction that has a close() method, as well
> as a way to transfer the resource to another owner (i.e. the NativeScope). Then we can have a register(Resource) method
> on NativeScope that transfers the resource to the scope. One of the problems currently is that NativeScope doesn't call
> the close() method of VaList, but the close() method of the MemorySegment that we allocate through it, so we have to
> piggy back on the current NativeScope capabilities, or create an internal-only way of registering a VaList with it (the
> latter feels kinda hacky again).

I see FWIW I don't consider it that terrible to create a segment such as on construction
MemorySegment segment = MemorySegment.ofNativeRestricted(addr, Long.MAX_VALUE, Thread.currentThread(), null, null);
or on copy:
MemorySegment segment = MemorySegment.ofNativeRestricted(ptr, Long.MAX_VALUE, Thread.currentThread(), null, null);
for such purposes. It's reasonably self contained and requires no new abstraction. I do agree its awkward to treat this
as a "unit", but if this problem is just constrained to va_list it may be ok.

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

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