8357847: (ch) WindowsAsynchronousFileChannelImpl should support FFM Buffers

[bplb]

Acquire the scope of a direct buffer before it is used and release it after the task has finished with it, whether the task execution is immediate or pended.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8357847: (ch) WindowsAsynchronousFileChannelImpl should support FFM Buffers (Bug - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25531/head:pull/25531
$ git checkout pull/25531

Update a local copy of the PR:
$ git checkout pull/25531
$ git pull https://git.openjdk.org/jdk.git pull/25531/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25531

View PR using the GUI difftool:
$ git pr show -t 25531

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25531.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@bplb The following label will be automatically applied to this pull request:

• nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (82fb9e0b)
• 00: Full (10000234)

[AlanBateman]

@bplb This will need tests to exercise the read/write methods with buffers that a views on memory segments allocated from automatic and shared arenas. I think we also need to check that we have tests to ensure that UOE is thrown for thread confined arenas.

[AlanBateman]

I assume we'll have to change this to use IOUtil.bufferAddress(dst), and after the acquire.

[bplb]

Noted.

[bplb]

I don't think that using IOUtil.bufferAddress() would be correct here. The reason is that it calls JavaNioAccess.getBufferAddress() which returns the value of address hoisted into Buffer thereby circumventing the check for closable shared sessions mentioned below.

[bplb]

his will need tests to exercise the read/write methods with buffers that a views on memory segments allocated from automatic and shared arenas.

There is already testing on automatic arenas, I believe, and passing in a shared arena resulted in a UOE.

I think we also need to check that we have tests to ensure that UOE is thrown for thread confined arenas.

I observed this UOE already so that should be simple.

[AlanBateman]

There is already testing on automatic arenas, I believe, and passing in a shared arena resulted in a UOE.

We should be able to use a buffer that is a view of memory segment allocated from shared arena, only the confined case should throw UOE.

[bplb]

We should be able to use a buffer that is a view of memory segment allocated from shared arena, only the confined case should throw UOE.

I ran into this in testing:

    public long address() {
        MemorySessionImpl session = session();
        if (session != null) {
            if (session.ownerThread() == null && session.isCloseable()) {
                throw new UnsupportedOperationException("ByteBuffer derived from closeable shared sessions not supported");
            }

in Direct-X-Buffer.java.template when using the result of Arena.ofShared.

[AlanBateman]

I ran into this in testing:

    public long address() {
        MemorySessionImpl session = session();
        if (session != null) {
            if (session.ownerThread() == null && session.isCloseable()) {
                throw new UnsupportedOperationException("ByteBuffer derived from closeable shared sessions not supported");
            }

in Direct-X-Buffer.java.template when using the result of Arena.ofShared.

My previous comment wasn't correct, the above code where it checks isCloseable is correct. We should be able to use a buffer from a view allocated from the global arena or an automatic arena. It should throw UOE for confined and shared.

[bplb]

We should be able to use a buffer from a view allocated from the global arena or an automatic arena. It should throw UOE for confined and shared.

Currently genBuffer in java/nio/channels/AsynchronousFileChannel/Basic.java has this case:

            case 2 -> Arena.ofAuto().allocate(buf.length).asByteBuffer()
                    .put(buf)
                    .flip();

Other cases could be added there, e.g.,

            case 2 -> Arena.global().allocate(buf.length).asByteBuffer()
                    .put(buf)
                    .flip();

The UOE for confined and shared might have to be checked elsewhere. I'll examine it further.

[bplb]

We should be able to use a buffer from a view allocated from the global arena or an automatic arena. It should throw UOE for confined and shared.

It throws IllegalStateException for confined and UOE for shared. I have broadened testing in 82fb9e0.