On Tue, 29 Nov 2022 09:40:58 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> This PR proposes the introduction of **guarding** of the use of >> `DirectBuffer::address` within the JDK. With the introduction of the Foreign >> Function and Memory access, it is possible to derive Buffer instances that >> are backed by native memory that, in turn, can be closed asynchronously by >> the user (and not only via a `Cleaner` when there is no other reference to >> the `Buffer` object). If another thread is invoking `MemorySession::close` >> while a thread is doing work using raw addresses, the outcome is undefined. >> This means the JVM might crash or even worse, silent modification of >> unrelated memory might occur. >> >> Design choices in this PR: >> >> There is already a method `MemorySession::whileAlive` that takes a runnable >> and that will perform the provided action while acquiring the underlying` >> MemorySession` (if any). However, using this method entailed relatively >> large changes while converting larger/nested code segments into lambdas. >> This would also risk introducing lambda capturing. So, instead, a >> ~~try-with-resources~~ *try-finally* friendly access method was added. This >> made is more easy to add guarding and did not risk lambda capturing. Also, >> introducing lambdas in certain fundamental JDK classes might incur bootstrap >> problems. >> >> The aforementioned ~~TwR~~ TF is using a ~~"session acquisition" that is not >> used explicitly in the try block itself~~ session used in the *finally* >> block. ~~This raises a warning that is suppressed using >> `@SuppressWarnings("try")`. In the future, we might be able to remove these >> suppressions by using the reserved variable name `_`.~~ >> >> In some cases, where is is guaranteed that the backing memory session is >> non-closeable, we do not have to guard the use of `DirectBuffer::address`. >> ~~These cases have been documented in the code.~~ >> >> On some occasions, a plurality of acquisitions are made. This would never >> lead to deadlocks as acquisitions are fundamentally concurrent counters and >> not resources that only one thread can "own". >> >> I have added comments (and not javadocs) before the declaration of the >> non-public-api `DirectBuffer::address` method, that the use of the returned >> address needs to be guarded. It can be discussed if this is preferable or >> not. >> >> This PR spawns several areas of responsibility and so, I expect more than >> one reviewer before promoting the PR. > > Per Minborg has updated the pull request incrementally with one additional > commit since the last revision: > > Remove session exposure Marked as reviewed by bpb (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/11260