RFR: 8296024: Usage of DIrectBuffer::address should be guarded [v4]

[Per Minborg]

On Mon, 21 Nov 2022 14:07:38 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename methods
>
> src/java.base/share/classes/java/util/zip/Adler32.java line 105:
> 
>> 103:                 adler = updateByteBuffer(adler, ((DirectBuffer)buffer).address(), pos, rem);
>> 104:             } finally {
>> 105:                 Reference.reachabilityFence(buffer);
> 
> The updated naming is a bit better but there are it still feels that that there are too many things going on at the use sites ("guard", "session", "reachability fence"). Ideally the acquire would return something with an accessor for the direct buffer address but that may not be possible.  I wonder if changing NOP_CLOSE.close to do reachabilityFence(this) would work as expected and improve many of these use sites?

This can certainly be done. We could, for example, add a new method to the `JavaNioAccess` interface:

```AddressAcquisition acquireSession2(Buffer buffer); // to be renamed```

This would allow us to go from:


            try (var guard = NIO_ACCESS.acquireSession(buffer)) {
                adler = updateByteBuffer(adler, ((DirectBuffer)buffer).address(), pos, rem);
            } finally {
                Reference.reachabilityFence(buffer);
            }


to


            try (var guard = NIO_ACCESS.acquireSession2(buffer)) {
                adler = updateByteBuffer(adler, guard.address(), pos, rem);
            }


Although this looks much simpler and concise, it means "a new object is created for each invocation" (*). This also allows us to remove the `@SupressWarnings("try")` annotations.

Here is how the undercarriage might look like:


                @Override
                public AddressAcquisition acquireSession2(Buffer buffer) {
                    var session = buffer.session();
                    if (session == null) {
                        return AddressAcquisition.create(buffer, () -> {});
                    }
                    session.acquire0();
                    return AddressAcquisition.create(buffer, session::release0);
                }



and


sealed interface AddressAcquisition extends AutoCloseable {

        long address();

        @Override
        void close();

        static AddressAcquisition create(Buffer buffer, Runnable closeActions) {
            return new AddressAcquisitionImpl(buffer, closeActions);
        }

        final class AddressAcquisitionImpl implements AddressAcquisition {

            private final DirectBuffer directBuffer;
            private final Runnable closeAction;

            public AddressAcquisitionImpl(Buffer buffer,
                                          Runnable closeAction) {
                this.directBuffer = (DirectBuffer) buffer;
                this.closeAction = closeAction;
            }

            @Override
            public long address() {
                return directBuffer.address();
            }

            @Override
            public void close() {
                try {
                    closeAction.run();
                } finally {
                    Reference.reachabilityFence(directBuffer);
                }
            }
        }

    }



(*) In reality,  a representation of the object might be allocated on the stack instead.

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

PR: https://git.openjdk.org/jdk/pull/11260