On Mon, 21 Nov 2022 11:32:35 GMT, Alan Bateman <al...@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 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 is using a "session acquisition" that is not used >> explicitly in the try block itself. 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 `_`. Another >> alternative was evaluated where a non-autocloseable resource was used. >> However, it became relatively complicated to guarantee that the session was >> always released and, at the same time, the existing 'final` block was always >> executed properly (as they both might throw exceptions). In the end, the >> proposed solution was much simpler and robust despite requiring a >> non-referenced TwR variable. >> >> 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. > > src/java.base/share/classes/java/util/zip/Adler32.java line 102: > >> 100: return; >> 101: if (buffer.isDirect()) { >> 102: try (var sessionAcquisition = >> NIO_ACCESS.acquireSessionAsAutoCloseable(buffer)) { > > We need to find something better than "sessionAcquisition", it looks very > messy at all these use sites. Eventually, I hope most of them will be named `_`. ------------- PR: https://git.openjdk.org/jdk/pull/11260