> 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.

Per Minborg has updated the pull request incrementally with one additional 
commit since the last revision:

  Reformat and fix comment

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/11260/files
  - new: https://git.openjdk.org/jdk/pull/11260/files/0546b23b..30aff118

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=11260&range=13
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11260&range=12-13

  Stats: 26 lines in 3 files changed: 13 ins; 2 del; 11 mod
  Patch: https://git.openjdk.org/jdk/pull/11260.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11260/head:pull/11260

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

Reply via email to